regex perf improvements

proxy_agent/src/common/config.rs

@@ -122,39 +122,44 @@ impl Default for Config {
             config_file_full_path = misc_helpers::get_current_exe_dir();
             config_file_full_path.push(CONFIG_FILE_NAME);
         }
+
         Config::from_json_file(config_file_full_path)
     }
 }
 
 impl Config {
+    /// Load config from a specific JSON file path
     pub fn from_json_file(file_path: PathBuf) -> Self {
-        misc_helpers::json_read_from_file::<Config>(&file_path).unwrap_or_else(|_| {
-            panic!(
-                "Error in reading Config from Json file: {}",
-                misc_helpers::path_to_string(&file_path)
-            )
-        })
+        let mut config =
+            misc_helpers::json_read_from_file::<Config>(&file_path).unwrap_or_else(|_| {
+                panic!(
+                    "Error in reading Config from Json file: {}",
+                    misc_helpers::path_to_string(&file_path)
+                )
+            });
+        config.resolve_env_variables();
+        config
+    }
+
+    /// Resolve environment variables in path fields once during construction
+    /// This allows us to keep the rest of the code simple without worrying about env vars,
+    /// and also ensures that we only pay the cost of resolving env vars once at startup rather than on every access.
+    fn resolve_env_variables(&mut self) {
+        self.logFolder = misc_helpers::resolve_env_variables(&self.logFolder);
+        self.eventFolder = misc_helpers::resolve_env_variables(&self.eventFolder);
+        self.latchKeyFolder = misc_helpers::resolve_env_variables(&self.latchKeyFolder);
     }
 
     pub fn get_log_folder(&self) -> String {
-        match misc_helpers::resolve_env_variables(&self.logFolder) {
-            Ok(val) => val,
-            Err(_) => self.logFolder.clone(),
-        }
+        self.logFolder.clone()
     }
 
     pub fn get_event_folder(&self) -> String {
-        match misc_helpers::resolve_env_variables(&self.eventFolder) {
-            Ok(val) => val,
-            Err(_) => self.eventFolder.clone(),
-        }
+        self.eventFolder.clone()
     }
 
     pub fn get_latch_key_folder(&self) -> String {
-        match misc_helpers::resolve_env_variables(&self.latchKeyFolder) {
-            Ok(val) => val,
-            Err(_) => self.latchKeyFolder.clone(),
-        }
+        self.latchKeyFolder.clone()
     }
 
     pub fn get_monitor_interval(&self) -> u64 {
@@ -231,19 +236,19 @@ mod tests {
         let config = create_config_file(config_file_path);
 
         assert_eq!(
-            r#"C:\logFolderName"#.to_string(),
+            r#"C:\logFolderName"#,
             config.get_log_folder(),
             "Log Folder mismatch"
         );
 
         assert_eq!(
-            r#"C:\eventFolderName"#.to_string(),
+            r#"C:\eventFolderName"#,
             config.get_event_folder(),
             "Event Folder mismatch"
         );
 
         assert_eq!(
-            r#"C:\latchKeyFolderName"#.to_string(),
+            r#"C:\latchKeyFolderName"#,
             config.get_latch_key_folder(),
             "Latch Key Folder mismatch"
         );
@@ -267,7 +272,7 @@ mod tests {
         );
 
         assert_eq!(
-            "ebpfProgramName".to_string(),
+            "ebpfProgramName",
             config.get_ebpf_program_name(),
             "get_ebpf_program_name mismatch"
         );

proxy_agent/src/proxy.rs

@@ -200,7 +200,7 @@ impl Process {
         }
 
         // redact the secrets in the command line
-        let cmd = proxy_agent_shared::secrets_redactor::redact_secrets(cmd);
+        let cmd = proxy_agent_shared::secrets_redactor::redact_secrets_string(cmd);
 
         let process_name = process_full_path
             .file_name()

proxy_agent/src/proxy/authorization_rules.rs

@@ -263,6 +263,10 @@ pub struct AuthorizationRulesForLogging {
     pub computedRules: ComputedAuthorizationRules,
 }
 
+/// Remark: Regex::new is performance-sensitive, so we use LazyLock to compile it only once and reuse it for subsequent calls
+static AUTHORIZATION_RULES_FILE_SEARCH_REGEX: std::sync::LazyLock<regex::Regex> =
+    std::sync::LazyLock::new(|| regex::Regex::new(r"^AuthorizationRules_.*\.json$").unwrap());
+
 impl AuthorizationRulesForLogging {
     pub fn new(
         input_rules: Option<AuthorizationRules>,
@@ -280,7 +284,10 @@ impl AuthorizationRulesForLogging {
     /// The file is written to the path_dir specified by the input parameter
     pub fn write_all(&self, path_dir: &Path, max_file_count: usize) {
         // remove the old files
-        let files = match misc_helpers::search_files(path_dir, r"^AuthorizationRules_.*\.json$") {
+        let files = match misc_helpers::search_files(
+            path_dir,
+            &AUTHORIZATION_RULES_FILE_SEARCH_REGEX,
+        ) {
             Ok(files) => files,
             Err(e) => {
                 // This should not happen, log the error and skip write the file
@@ -343,7 +350,9 @@ mod tests {
         AccessControlRules, AuthorizationItem, AuthorizationRules, Identity, Privilege, Role,
         RoleAssignment,
     };
-    use crate::proxy::authorization_rules::{AuthorizationMode, ComputedAuthorizationItem};
+    use crate::proxy::authorization_rules::{
+        AuthorizationMode, ComputedAuthorizationItem, AUTHORIZATION_RULES_FILE_SEARCH_REGEX,
+    };
     use crate::proxy::{proxy_connection::ConnectionLogger, Claims};
     use proxy_agent_shared::misc_helpers;
     use std::ffi::OsString;
@@ -600,7 +609,8 @@ mod tests {
         }
 
         let files =
-            misc_helpers::search_files(&temp_test_path, r"^AuthorizationRules_.*\.json$").unwrap();
+            misc_helpers::search_files(&temp_test_path, &AUTHORIZATION_RULES_FILE_SEARCH_REGEX)
+                .unwrap();
         assert_eq!(files.len(), max_file_count);
 
         // clean up and ignore the clean up errors

proxy_agent_setup/src/running.rs

@@ -26,8 +26,7 @@ pub fn proxy_agent_running_folder(_service_name: &str) -> PathBuf {
 pub fn proxy_agent_parent_folder() -> PathBuf {
     #[cfg(windows)]
     {
-        let path = misc_helpers::resolve_env_variables("%SYSTEMDRIVE%\\WindowsAzure\\ProxyAgent")
-            .unwrap_or("C:\\WindowsAzure\\ProxyAgent".to_string());
+        let path = misc_helpers::resolve_env_variables("%SYSTEMDRIVE%\\WindowsAzure\\ProxyAgent");
         PathBuf::from(path)
     }
     #[cfg(not(windows))]

proxy_agent_shared/src/etw/etw_writter.rs

@@ -45,7 +45,7 @@ impl WindowsEventWritter {
         );
         let value = crate::misc_helpers::resolve_env_variables(
             r"%SystemRoot%\Microsoft.NET\Framework64\v4.0.30319\EventLogMessages.dll",
-        )?;
+        );
         crate::windows::set_reg_string(&key_name, "EventMessageFile", value)?;
 
         let source_name_wide = super::to_wide(source_name);

proxy_agent_shared/src/misc_helpers.rs

@@ -261,28 +261,18 @@ pub fn empty_path() -> PathBuf {
     PathBuf::new()
 }
 
-/// Search files in a directory with a regex pattern
+/// Search files in a directory with a regex
 /// # Arguments
 /// * `dir` - The directory to search
-/// * `search_regex_pattern` - The regex pattern to search
+/// * `search_regex` - The regex to search
 /// # Returns
 /// A vector of PathBufs that match the search pattern in ascending order
 /// # Errors
 /// Returns an error if the regex pattern is invalid or if there is an IO error
-/// # Example
-/// ```rust
-/// use std::path::PathBuf;
-/// use proxy_agent_shared::misc_helpers;
-/// let dir = PathBuf::from(".");
-/// let search_regex_pattern = r"^(.*\.log)$";  // search for files with .log extension
-/// let files = misc_helpers::search_files(&dir, search_regex_pattern).unwrap();
-///
-/// let search_regex_pattern = r"^MyFile.*\.json$"; // Regex pattern to match "MyFile*.json"
-/// let files = misc_helpers::search_files(&dir, search_regex_pattern).unwrap();
-/// ```
-pub fn search_files(dir: &Path, search_regex_pattern: &str) -> Result<Vec<PathBuf>> {
+/// Remarks: The Regex::new is expensive, so the caller should cache the regex if it is used frequently,
+///     for example, by using once_cell::sync::Lazy or std::sync::LazyLock to create a static regex instance.
+pub fn search_files(dir: &Path, search_regex: &Regex) -> Result<Vec<PathBuf>> {
     let mut files = Vec::new();
-    let regex = Regex::new(search_regex_pattern)?;
 
     for entry in fs::read_dir(dir)? {
         let entry = entry?;
@@ -292,7 +282,7 @@ pub fn search_files(dir: &Path, search_regex_pattern: &str) -> Result<Vec<PathBu
             continue;
         }
         let file_name = get_file_name(&file_full_path);
-        if regex.is_match(&file_name) {
+        if search_regex.is_match(&file_name) {
             files.push(file_full_path);
         }
     }
@@ -378,21 +368,32 @@ pub fn get_proxy_agent_version(proxy_agent_exe: &Path) -> Result<String> {
     }
 }
 
+/// Static regex for matching environment variables like %VAR%
+/// expect() only panics on failure to compile the regex, which should not happen since the pattern is constant and valid
+/// This is the idiomatic Rust pattern for creating a static regex that is compiled once and reused, ensuring thread safety and performance
+/// Remark: Regex::new is performance-sensitive, so we use LazyLock to compile it only once and reuse it for subsequent calls to resolve_env_variables, which can be called frequently.
+///     This avoids the overhead of compiling the regex on every call, improving performance while ensuring thread safety.
+static ENV_VAR_REGEX: std::sync::LazyLock<Regex> =
+    std::sync::LazyLock::new(|| Regex::new(r"%(\w+)%").expect("Invalid env var regex pattern"));
+
 /// This function replaces all occurrences of %VAR% in the input string with the value of the environment variable VAR
 /// If the environment variable is not set, it returns the original string with VAR unchanged.
 /// # Arguments
 /// * `input` - The input string to resolve environment variables in
 /// # Returns
 /// A Result containing the resolved string or an error if the regex pattern is invalid
-pub fn resolve_env_variables(input: &str) -> Result<String> {
-    let re = Regex::new(r"%(\w+)%")?;
-    let ret = re
+/// The resolved string with environment variables expanded
+pub fn resolve_env_variables(input: &str) -> String {
+    if input.is_empty() || !input.contains('%') {
+        // If the input string is empty or does not contain '%', return the original string
+        return input.to_string();
+    }
+
+    ENV_VAR_REGEX
         .replace_all(input, |caps: &regex::Captures| {
             std::env::var(&caps[1]).unwrap_or_else(|_| caps[1].to_string())
         })
-        .to_string();
-
-    Ok(ret)
+        .to_string()
 }
 
 /// Compute HMAC-SHA256 signature for the input using the provided hex-encoded key
@@ -424,6 +425,7 @@ pub fn xml_escape(s: String) -> String {
 
 #[cfg(test)]
 mod tests {
+    use regex::Regex;
     use serde_derive::{Deserialize, Serialize};
     use std::env;
     use std::fs;
@@ -578,14 +580,16 @@ mod tests {
         let json_file = json_file.join("test_1.json");
         super::json_write_to_file(&test, &json_file).unwrap();
 
-        let files = super::search_files(&temp_test_path, "test.json").unwrap();
+        let regex = Regex::new(r"test.json").unwrap();
+        let files = super::search_files(&temp_test_path, &regex).unwrap();
         assert_eq!(
             1,
             files.len(),
             "file count mismatch with 'test.json' search"
         );
 
-        let files = super::search_files(&temp_test_path, r"^test.*\.json$").unwrap();
+        let regex = Regex::new(r"^test.*\.json$").unwrap();
+        let files = super::search_files(&temp_test_path, &regex).unwrap();
         assert_eq!(
             2,
             files.len(),
@@ -647,12 +651,12 @@ mod tests {
             "{}\\WindowsAzure\\ProxyAgent\\Package_1.0.0",
             env::var("SYSTEMDRIVE").unwrap_or("SYSTEMDRIVE".to_string())
         );
-        let resolved = super::resolve_env_variables(input).unwrap();
+        let resolved = super::resolve_env_variables(input);
         assert_eq!(expected, resolved, "resolved string mismatch");
 
         let input = "/var/log/azure-proxy-agent/";
         let expected = "/var/log/azure-proxy-agent/".to_string();
-        let resolved = super::resolve_env_variables(input).unwrap();
+        let resolved = super::resolve_env_variables(input);
         assert_eq!(expected, resolved, "resolved string mismatch");
     }
 

proxy_agent_shared/src/proxy_agent_aggregate_status.rs

@@ -12,8 +12,7 @@ const PROXY_AGENT_AGGREGATE_STATUS_FOLDER: &str = "/var/log/azure-proxy-agent/";
 pub const PROXY_AGENT_AGGREGATE_STATUS_FILE_NAME: &str = "status.json";
 
 pub fn get_proxy_agent_aggregate_status_folder() -> std::path::PathBuf {
-    let path = misc_helpers::resolve_env_variables(PROXY_AGENT_AGGREGATE_STATUS_FOLDER)
-        .unwrap_or(PROXY_AGENT_AGGREGATE_STATUS_FOLDER.to_string());
+    let path = misc_helpers::resolve_env_variables(PROXY_AGENT_AGGREGATE_STATUS_FOLDER);
     PathBuf::from(path)
 }