feat(en): Brush up EN observability config

core/bin/external_node/src/config/mod.rs

@@ -1,5 +1,6 @@
 use std::{
     env,
+    ffi::OsString,
     num::{NonZeroU32, NonZeroU64, NonZeroUsize},
     time::Duration,
 };
@@ -34,14 +35,40 @@ use zksync_web3_decl::{
     namespaces::{EnNamespaceClient, ZksNamespaceClient},
 };
 
+use crate::config::observability::ObservabilityENConfig;
+
 pub(crate) mod observability;
 #[cfg(test)]
 mod tests;
 
 const BYTES_IN_MEGABYTE: usize = 1_024 * 1_024;
 
+/// Encapsulation of configuration source with a mock implementation used in tests.
+trait ConfigurationSource: 'static {
+    type Vars<'a>: Iterator<Item = (OsString, OsString)> + 'a;
+
+    fn vars(&self) -> Self::Vars<'_>;
+
+    fn var(&self, name: &str) -> Option<String>;
+}
+
+#[derive(Debug)]
+struct Environment;
+
+impl ConfigurationSource for Environment {
+    type Vars<'a> = env::VarsOs;
+
+    fn vars(&self) -> Self::Vars<'_> {
+        env::vars_os()
+    }
+
+    fn var(&self, name: &str) -> Option<String> {
+        env::var(name).ok()
+    }
+}
+
 /// This part of the external node config is fetched directly from the main node.
-#[derive(Debug, Deserialize, Clone, PartialEq)]
+#[derive(Debug, Deserialize)]
 pub(crate) struct RemoteENConfig {
     pub bridgehub_proxy_addr: Option<Address>,
     pub state_transition_proxy_addr: Option<Address>,
@@ -165,7 +192,7 @@ impl RemoteENConfig {
     }
 }
 
-#[derive(Debug, Deserialize, Clone, PartialEq)]
+#[derive(Debug, Deserialize)]
 pub(crate) enum BlockFetcher {
     ServerAPI,
     Consensus,
@@ -174,7 +201,7 @@ pub(crate) enum BlockFetcher {
 /// This part of the external node config is completely optional to provide.
 /// It can tweak limits of the API, delay intervals of certain components, etc.
 /// If any of the fields are not provided, the default values will be used.
-#[derive(Debug, Clone, PartialEq, Deserialize)]
+#[derive(Debug, Deserialize)]
 pub(crate) struct OptionalENConfig {
     // User-facing API limits
     /// Max possible limit of filters to be in the API state at once.
@@ -315,8 +342,6 @@ pub(crate) struct OptionalENConfig {
     database_slow_query_threshold_ms: Option<u64>,
 
     // Other config settings
-    /// Port on which the Prometheus exporter server is listening.
-    pub prometheus_port: Option<u16>,
     /// Capacity of the queue for asynchronous miniblock sealing. Once this many miniblocks are queued,
     /// sealing will block until some of the miniblocks from the queue are processed.
     /// 0 means that sealing is synchronous; this is mostly useful for performance comparison, testing etc.
@@ -606,7 +631,7 @@ impl OptionalENConfig {
 }
 
 /// This part of the external node config is required for its operation.
-#[derive(Debug, Deserialize, Clone, PartialEq)]
+#[derive(Debug, Deserialize)]
 pub(crate) struct RequiredENConfig {
     /// L1 chain ID (e.g., 9 for Ethereum mainnet). This ID will be checked against the `eth_client_url` RPC provider on initialization
     /// to ensure that there's no mismatch between the expected and actual L1 network.
@@ -664,7 +689,7 @@ impl RequiredENConfig {
 /// While also mandatory, it historically used different naming scheme for corresponding
 /// environment variables.
 /// Thus it is kept separately for backward compatibility and ease of deserialization.
-#[derive(Debug, Clone, Deserialize, PartialEq)]
+#[derive(Debug, Deserialize)]
 pub(crate) struct PostgresConfig {
     database_url: SensitiveUrl,
     pub max_connections: u32,
@@ -699,7 +724,7 @@ impl PostgresConfig {
 
 /// Experimental part of the external node config. All parameters in this group can change or disappear without notice.
 /// Eventually, parameters from this group generally end up in the optional group.
-#[derive(Debug, Clone, Deserialize)]
+#[derive(Debug, Deserialize)]
 pub(crate) struct ExperimentalENConfig {
     // State keeper cache config
     /// Block cache capacity of the state keeper RocksDB cache. The default value is 128 MB.
@@ -767,26 +792,27 @@ impl SnapshotsRecoveryConfig {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Deserialize)]
+#[derive(Debug, Deserialize)]
 pub struct ApiComponentConfig {
     /// Address of the tree API used by this EN in case it does not have a
     /// local tree component running and in this case needs to send requests
     /// to some external tree API.
     pub tree_api_remote_url: Option<String>,
 }
 
-#[derive(Debug, Clone, PartialEq, Deserialize)]
+#[derive(Debug, Deserialize)]
 pub struct TreeComponentConfig {
     pub api_port: Option<u16>,
 }
 
 /// External Node Config contains all the configuration required for the EN operation.
 /// It is split into three parts: required, optional and remote for easier navigation.
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub(crate) struct ExternalNodeConfig {
     pub required: RequiredENConfig,
     pub postgres: PostgresConfig,
     pub optional: OptionalENConfig,
+    pub observability: ObservabilityENConfig,
     pub remote: RemoteENConfig,
     pub experimental: ExperimentalENConfig,
     pub consensus: Option<ConsensusConfig>,
@@ -799,6 +825,7 @@ impl ExternalNodeConfig {
     pub async fn new(
         required: RequiredENConfig,
         optional: OptionalENConfig,
+        observability: ObservabilityENConfig,
         main_node_client: &BoxedL2Client,
     ) -> anyhow::Result<Self> {
         let experimental = envy::prefixed("EN_EXPERIMENTAL_")
@@ -823,6 +850,7 @@ impl ExternalNodeConfig {
             required,
             optional,
             experimental,
+            observability,
             consensus: read_consensus_config().context("read_consensus_config()")?,
             tree_component: tree_component_config,
             api_component: api_component_config,
@@ -836,6 +864,7 @@ impl ExternalNodeConfig {
             postgres: PostgresConfig::mock(test_pool),
             optional: OptionalENConfig::mock(),
             remote: RemoteENConfig::mock(),
+            observability: ObservabilityENConfig::default(),
             experimental: ExperimentalENConfig::mock(),
             consensus: None,
             api_component: ApiComponentConfig {
@@ -846,8 +875,8 @@ impl ExternalNodeConfig {
     }
 }
 
-impl From<ExternalNodeConfig> for InternalApiConfig {
-    fn from(config: ExternalNodeConfig) -> Self {
+impl From<&ExternalNodeConfig> for InternalApiConfig {
+    fn from(config: &ExternalNodeConfig) -> Self {
         Self {
             l1_chain_id: config.required.l1_chain_id,
             l2_chain_id: config.required.l2_chain_id,
@@ -879,8 +908,8 @@ impl From<ExternalNodeConfig> for InternalApiConfig {
     }
 }
 
-impl From<ExternalNodeConfig> for TxSenderConfig {
-    fn from(config: ExternalNodeConfig) -> Self {
+impl From<&ExternalNodeConfig> for TxSenderConfig {
+    fn from(config: &ExternalNodeConfig) -> Self {
         Self {
             // Fee account address does not matter for the EN operation, since
             // actual fee distribution is handled my the main node.

core/bin/external_node/src/config/observability.rs

@@ -1,43 +1,101 @@
-use zksync_config::configs::ObservabilityConfig;
-
-pub fn observability_config_from_env() -> anyhow::Result<ObservabilityConfig> {
-    // The logic in this method mimics the historical logic of loading observability options
-    // This is left intact, since some of the existing deployments may rely on the this behavior.
-    let sentry_url = if let Ok(sentry_url) = std::env::var("MISC_SENTRY_URL") {
-        if sentry_url == "unset" {
-            None
-        } else {
-            Some(sentry_url)
+use std::{collections::HashMap, time::Duration};
+
+use anyhow::Context as _;
+use prometheus_exporter::PrometheusExporterConfig;
+use serde::Deserialize;
+use vlog::LogFormat;
+
+use super::{ConfigurationSource, Environment};
+
+/// Observability part of the node configuration.
+#[derive(Debug, Default, Deserialize)]
+pub(crate) struct ObservabilityENConfig {
+    /// Port to bind the Prometheus exporter server to. If not specified, the server will not be launched.
+    /// If the push gateway URL is specified, it will prevail.
+    pub prometheus_port: Option<u16>,
+    /// Prometheus push gateway to push metrics to. Overrides `prometheus_port`. A full URL must be specified
+    /// including `job_id` and other path segments; it will be used verbatim as the URL to push data to.
+    pub prometheus_pushgateway_url: Option<String>,
+    /// Interval between pushing metrics to the Prometheus push gateway.
+    #[serde(default = "ObservabilityENConfig::default_prometheus_push_interval_ms")]
+    pub prometheus_push_interval_ms: u64,
+    /// Sentry URL to send panics to.
+    pub sentry_url: Option<String>,
+    /// Environment to use when sending data to Sentry.
+    pub sentry_environment: Option<String>,
+    /// Log format to use: either `plain` (default) or `json`.
+    #[serde(default)]
+    pub log_format: LogFormat,
+}
+
+impl ObservabilityENConfig {
+    const fn default_prometheus_push_interval_ms() -> u64 {
+        10_000
+    }
+
+    pub fn from_env() -> envy::Result<Self> {
+        Self::new(&Environment)
+    }
+
+    pub(super) fn new(source: &impl ConfigurationSource) -> envy::Result<Self> {
+        const OBSOLETE_VAR_NAMES: &[(&str, &str)] = &[
+            ("MISC_SENTRY_URL", "EN_SENTRY_URL"),
+            ("MISC_LOG_FORMAT", "EN_LOG_FORMAT"),
+        ];
+
+        let en_vars = source.vars().filter_map(|(name, value)| {
+            let name = name.into_string().ok()?;
+            if !name.starts_with("EN_") {
+                return None;
+            }
+            Some((name, value.into_string().ok()?))
+        });
+        let mut vars: HashMap<_, _> = en_vars.collect();
+
+        for &(old_name, new_name) in OBSOLETE_VAR_NAMES {
+            if vars.contains_key(new_name) {
+                continue; // new name is set; it should prevail over the obsolete one.
+            }
+            if let Some(value) = source.var(old_name) {
+                vars.insert(new_name.to_owned(), value);
+            }
         }
-    } else {
-        None
-    };
-    let sentry_environment = std::env::var("EN_SENTRY_ENVIRONMENT").ok().or_else(|| {
-        let l1_network = std::env::var("CHAIN_ETH_NETWORK").ok();
-        let l2_network = std::env::var("CHAIN_ETH_ZKSYNC_NETWORK").ok();
-        match (l1_network, l2_network) {
-            (Some(l1_network), Some(l2_network)) => {
-                Some(format!("{} - {}", l1_network, l2_network))
+
+        envy::prefixed("EN_").from_iter(vars)
+    }
+
+    pub fn prometheus(&self) -> Option<PrometheusExporterConfig> {
+        match (self.prometheus_port, &self.prometheus_pushgateway_url) {
+            (_, Some(url)) => {
+                if self.prometheus_port.is_some() {
+                    tracing::info!("Both Prometheus port and push gateway URLs are specified; the push gateway URL will be used");
+                }
+                let push_interval = Duration::from_millis(self.prometheus_push_interval_ms);
+                Some(PrometheusExporterConfig::push(url.clone(), push_interval))
             }
-            _ => None,
+            (Some(port), None) => Some(PrometheusExporterConfig::pull(port)),
+            (None, None) => None,
         }
-    });
-    let log_format = if let Ok(log_format) = std::env::var("MISC_LOG_FORMAT") {
-        if log_format != "plain" && log_format != "json" {
-            anyhow::bail!("MISC_LOG_FORMAT has an unexpected value {}", log_format);
+    }
+
+    pub fn build_observability(&self) -> anyhow::Result<vlog::ObservabilityGuard> {
+        let mut builder = vlog::ObservabilityBuilder::new().with_log_format(self.log_format);
+        // Some legacy deployments use `unset` as an equivalent of `None`.
+        let sentry_url = self.sentry_url.as_deref().filter(|&url| url != "unset");
+        if let Some(sentry_url) = sentry_url {
+            builder = builder
+                .with_sentry_url(sentry_url)
+                .context("Invalid Sentry URL")?
+                .with_sentry_environment(self.sentry_environment.clone());
+        }
+        let guard = builder.build();
+
+        // Report whether sentry is running after the logging subsystem was initialized.
+        if let Some(sentry_url) = sentry_url {
+            tracing::info!("Sentry configured with URL: {sentry_url}");
+        } else {
+            tracing::info!("No sentry URL was provided");
         }
-        log_format
-    } else {
-        "plain".to_string()
-    };
-    let log_directives = std::env::var("RUST_LOG").ok();
-
-    Ok(ObservabilityConfig {
-        sentry_url,
-        sentry_environment,
-        log_format,
-        opentelemetry: None,
-        sporadic_crypto_errors_substrs: vec![],
-        log_directives,
-    })
+        Ok(guard)
+    }
 }

core/bin/external_node/src/config/tests.rs

@@ -1,7 +1,71 @@
 //! Tests for EN configuration.
 
+use std::collections::HashMap;
+
+use assert_matches::assert_matches;
+
 use super::*;
 
+#[derive(Debug)]
+struct MockEnvironment(HashMap<&'static str, &'static str>);
+
+impl MockEnvironment {
+    pub fn new(vars: &[(&'static str, &'static str)]) -> Self {
+        Self(vars.iter().copied().collect())
+    }
+}
+
+impl ConfigurationSource for MockEnvironment {
+    type Vars<'a> = Box<dyn Iterator<Item = (OsString, OsString)> + 'a>;
+
+    fn vars(&self) -> Self::Vars<'_> {
+        Box::new(
+            self.0
+                .iter()
+                .map(|(&name, &value)| (OsString::from(name), OsString::from(value))),
+        )
+    }
+
+    fn var(&self, name: &str) -> Option<String> {
+        self.0.get(name).copied().map(str::to_owned)
+    }
+}
+
+#[test]
+fn parsing_observability_config() {
+    let mut env_vars = MockEnvironment::new(&[
+        ("EN_PROMETHEUS_PORT", "3322"),
+        ("MISC_SENTRY_URL", "https://example.com/"),
+        ("EN_SENTRY_ENVIRONMENT", "mainnet - mainnet2"),
+    ]);
+    let config = ObservabilityENConfig::new(&env_vars).unwrap();
+    assert_eq!(config.prometheus_port, Some(3322));
+    assert_eq!(config.sentry_url.unwrap(), "https://example.com/");
+    assert_eq!(config.sentry_environment.unwrap(), "mainnet - mainnet2");
+    assert_matches!(config.log_format, vlog::LogFormat::Plain);
+    assert_eq!(config.prometheus_push_interval_ms, 10_000);
+
+    env_vars.0.insert("MISC_LOG_FORMAT", "json");
+    let config = ObservabilityENConfig::new(&env_vars).unwrap();
+    assert_matches!(config.log_format, vlog::LogFormat::Json);
+
+    // If both the canonical and obsolete vars are specified, the canonical one should prevail.
+    env_vars.0.insert("EN_LOG_FORMAT", "plain");
+    env_vars
+        .0
+        .insert("EN_SENTRY_URL", "https://example.com/new");
+    let config = ObservabilityENConfig::new(&env_vars).unwrap();
+    assert_matches!(config.log_format, vlog::LogFormat::Plain);
+    assert_eq!(config.sentry_url.unwrap(), "https://example.com/new");
+}
+
+#[test]
+fn using_unset_sentry_url() {
+    let env_vars = MockEnvironment::new(&[("MISC_SENTRY_URL", "unset")]);
+    let config = ObservabilityENConfig::new(&env_vars).unwrap();
+    config.build_observability().unwrap();
+}
+
 #[test]
 fn parsing_optional_config_from_empty_env() {
     let config: OptionalENConfig = envy::prefixed("EN_").from_iter([]).unwrap();