fix(obs): honor profiling export contracts (#4433)

Co-authored-by: heihutu <heihutu@gmail.com>

crates/obs/src/telemetry/mod.rs

@@ -92,30 +92,33 @@ pub(crate) fn init_telemetry(config: &OtelConfig) -> Result<OtelGuard, Telemetry
         || config.metric_endpoint.as_deref().map(|s| !s.is_empty()).unwrap_or(false)
         || config.log_endpoint.as_deref().map(|s| !s.is_empty()).unwrap_or(false);
 
-    if has_obs {
+    let mut guard = if has_obs {
-        return otel::init_observability_http(config, logger_level, is_production);
+        otel::init_observability_http(config, logger_level, is_production)?
-    }
-
-    // ── Rule 2 & 3: Local logging (file or stdout) ────────────────────────────
-    // `init_local_logging` internally decides between file and stdout mode
-    // based on whether a log directory is configured.
-    //
-    // We check the environment variable here (rather than relying solely on the
-    // config struct) to honour dynamic overrides set after config construction.
-    let user_set_log_dir = get_env_opt_str(ENV_OBS_LOG_DIRECTORY);
-    let effective_config = if user_set_log_dir.as_deref().filter(|d| !d.is_empty()).is_some() {
-        // Environment variable is set: ensure the config reflects it so that
-        // `init_local_logging` picks up the value even if the struct was built
-        // before the env var was set.
-        std::borrow::Cow::Owned(OtelConfig {
-            log_directory: user_set_log_dir,
-            ..config.clone()
-        })
     } else {
-        std::borrow::Cow::Borrowed(config)
+        // ── Rule 2 & 3: Local logging (file or stdout) ────────────────────────
+        // `init_local_logging` internally decides between file and stdout mode
+        // based on whether a log directory is configured.
+        //
+        // We check the environment variable here (rather than relying solely on the
+        // config struct) to honour dynamic overrides set after config construction.
+        let user_set_log_dir = get_env_opt_str(ENV_OBS_LOG_DIRECTORY);
+        let effective_config = if user_set_log_dir.as_deref().filter(|d| !d.is_empty()).is_some() {
+            // Environment variable is set: ensure the config reflects it so that
+            // `init_local_logging` picks up the value even if the struct was built
+            // before the env var was set.
+            std::borrow::Cow::Owned(OtelConfig {
+                log_directory: user_set_log_dir,
+                ..config.clone()
+            })
+        } else {
+            std::borrow::Cow::Borrowed(config)
+        };
+
+        local::init_local_logging(&effective_config, logger_level, is_production)?
     };
 
-    local::init_local_logging(&effective_config, logger_level, is_production)
+    guard.profiling_agent = otel::init_profiler(config);
+    Ok(guard)
 }
 
 #[cfg(test)]

crates/obs/src/telemetry/otel.rs

@@ -65,7 +65,7 @@ use rustfs_config::{
 };
 use std::collections::HashMap;
 use std::{fs, io::IsTerminal, time::Duration};
-use tracing::info;
+use tracing::{info, warn};
 use tracing_error::ErrorLayer;
 use tracing_opentelemetry::{MetricsLayer, OpenTelemetryLayer};
 use tracing_subscriber::{Layer, fmt::format::FmtSpan, layer::SubscriberExt, util::SubscriberInitExt};
@@ -172,8 +172,6 @@ pub(super) fn init_observability_http(
     // ── Meter provider (HTTP) ─────────────────────────────────────────────────
     let meter_provider = build_meter_provider(&metric_ep, config, res.clone(), &service_name, use_stdout)?;
 
-    let profiling_agent = init_profiler(config);
-
     // ── Logger Logic ──────────────────────────────────────────────────────────
     // Logging is the only signal that may intentionally route to either OTLP
     // or local files depending on configuration completeness.
@@ -316,7 +314,7 @@ pub(super) fn init_observability_http(
         tracer_provider,
         meter_provider,
         logger_provider,
-        profiling_agent,
+        profiling_agent: None,
         tracing_guard,
         stdout_guard,
         cleanup_handle,
@@ -517,7 +515,7 @@ fn build_logger_provider(
     feature = "pyroscope",
     any(target_os = "macos", all(target_os = "linux", target_env = "gnu", target_arch = "x86_64"))
 ))]
-fn init_profiler(config: &OtelConfig) -> Option<ProfilingAgent> {
+pub(super) fn init_profiler(config: &OtelConfig) -> Option<ProfilingAgent> {
     use pyroscope::backend::{BackendConfig, PprofConfig, pprof_backend};
     use pyroscope::pyroscope::PyroscopeAgentBuilder;
     use rustfs_config::VERSION;
@@ -529,10 +527,19 @@ fn init_profiler(config: &OtelConfig) -> Option<ProfilingAgent> {
         return None;
     }
 
-    let endpoint = config.profiling_endpoint.as_ref()?.as_str();
+    let Some(endpoint) = config
-    if endpoint.is_empty() {
+        .profiling_endpoint
+        .as_deref()
+        .map(str::trim)
+        .filter(|endpoint| !endpoint.is_empty())
+    else {
+        warn!(
+            backend = "pyroscope",
+            result = "profiling_endpoint_missing",
+            "Profiling export is enabled but no profiling endpoint was configured"
+        );
         return None;
-    }
+    };
 
     // Configure Pyroscope Agent
     let backend = pprof_backend(PprofConfig::default(), BackendConfig::default());
@@ -540,15 +547,33 @@ fn init_profiler(config: &OtelConfig) -> Option<ProfilingAgent> {
     let version = config.service_version.as_deref().unwrap_or(VERSION);
     let sample_rate = 100; // 100 Hz
 
-    let agent = PyroscopeAgentBuilder::new(endpoint, service_name, sample_rate, "pyroscope-rs", "1.0.1", backend)
+    let agent = match PyroscopeAgentBuilder::new(endpoint, service_name, sample_rate, "pyroscope-rs", "1.0.1", backend)
         .tags(vec![("version", version), ("profile_type", "cpu")])
         .build()
-        .ok()?;
+    {
+        Ok(agent) => agent,
+        Err(err) => {
+            warn!(
+                backend = "pyroscope",
+                endpoint,
+                result = "profiling_agent_build_failed",
+                error = %err,
+                "Profiling export agent initialization failed"
+            );
+            return None;
+        }
+    };
 
     match agent.start() {
         Ok(agent) => Some(agent),
         Err(err) => {
-            eprintln!("Pyroscope agent start error: {err:?}");
+            warn!(
+                backend = "pyroscope",
+                endpoint,
+                result = "profiling_agent_start_failed",
+                error = ?err,
+                "Profiling export agent failed to start"
+            );
             None
         }
     }
@@ -558,7 +583,18 @@ fn init_profiler(config: &OtelConfig) -> Option<ProfilingAgent> {
     feature = "pyroscope",
     any(target_os = "macos", all(target_os = "linux", target_env = "gnu", target_arch = "x86_64"))
 )))]
-fn init_profiler(_config: &OtelConfig) -> Option<ProfilingAgent> {
+pub(super) fn init_profiler(config: &OtelConfig) -> Option<ProfilingAgent> {
+    if config
+        .profiling_export_enabled
+        .unwrap_or(rustfs_config::DEFAULT_OBS_PROFILING_EXPORT_ENABLED)
+    {
+        warn!(
+            backend = "pyroscope",
+            result = "profiling_feature_not_compiled",
+            required_feature = "pyroscope",
+            "Profiling export is enabled but this binary was built without Pyroscope support"
+        );
+    }
     None
 }
 

rustfs/Cargo.toml

@@ -56,7 +56,7 @@ sftp = ["rustfs-protocols/sftp"]
 license = []
 io-scheduler-debug = []  # Enable debug information in I/O scheduler
 tracing-chunk-debug = []  # Enable per-chunk tracing in data plane (high noise, for debugging only)
-full = ["metrics-gpu", "ftps", "swift", "webdav", "sftp"]
+full = ["metrics-gpu", "ftps", "swift", "webdav", "sftp", "pyroscope"]
 manual-test-runners = []
 rio-v2 = ["rustfs-ecstore/rio-v2"]
 pyroscope = ["rustfs-obs/pyroscope"]

rustfs/src/profiling.rs

@@ -12,7 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use tracing::{debug, info};
+use rustfs_config::{
+    ENV_CPU_DURATION_SECS, ENV_CPU_FREQ, ENV_CPU_INTERVAL_SECS, ENV_CPU_MODE, ENV_ENABLE_PROFILING, ENV_MEM_INTERVAL_SECS,
+    ENV_MEM_PERIODIC, ENV_OUTPUT_DIR,
+};
+use tracing::{debug, info, warn};
 
 const LOG_COMPONENT_PROFILING: &str = "profiling";
 const LOG_SUBSYSTEM_CPU: &str = "cpu";
@@ -23,7 +27,32 @@ const MEMORY_PPROF_UNSUPPORTED_REASON: &str = "mimalloc_memory_pprof_unsupported
 pub const LOCAL_CPU_PPROF_UNSUPPORTED_SUMMARY: &str = "local CPU pprof dumps are not supported; use Pyroscope export instead";
 pub const MEMORY_PPROF_UNSUPPORTED_SUMMARY: &str = "memory pprof dumps are not supported with the mimalloc allocator";
 
+const LEGACY_PROFILING_ENV_KEYS: [&str; 8] = [
+    ENV_ENABLE_PROFILING,
+    ENV_CPU_MODE,
+    ENV_CPU_FREQ,
+    ENV_CPU_INTERVAL_SECS,
+    ENV_CPU_DURATION_SECS,
+    ENV_MEM_PERIODIC,
+    ENV_MEM_INTERVAL_SECS,
+    ENV_OUTPUT_DIR,
+];
+
 pub async fn init_from_env() {
+    let legacy_keys = legacy_profiling_env_keys_present();
+    if !legacy_keys.is_empty() {
+        warn!(
+            component = LOG_COMPONENT_PROFILING,
+            subsystem = LOG_SUBSYSTEM_RUNTIME,
+            event = "profiling_legacy_env_ignored",
+            ignored_keys = %legacy_keys.join(", "),
+            recommended_enable_key = "RUSTFS_OBS_PROFILING_EXPORT_ENABLED",
+            recommended_endpoint_key = "RUSTFS_OBS_PROFILING_ENDPOINT",
+            output_dir_key = ENV_OUTPUT_DIR,
+            "Legacy local profiling environment variables are ignored; use observability profiling export settings instead"
+        );
+    }
+
     info!(
         component = LOG_COMPONENT_PROFILING,
         subsystem = LOG_SUBSYSTEM_RUNTIME,
@@ -88,6 +117,33 @@ fn unsupported_message(summary: &str) -> String {
     )
 }
 
+fn legacy_profiling_env_keys_present() -> Vec<&'static str> {
+    LEGACY_PROFILING_ENV_KEYS
+        .into_iter()
+        .filter(|key| std::env::var_os(key).is_some())
+        .collect()
+}
+
 fn target_env() -> &'static str {
     option_env!("CARGO_CFG_TARGET_ENV").unwrap_or("unknown")
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn legacy_profiling_env_keys_present_reports_legacy_overrides() {
+        temp_env::with_vars(
+            [
+                (ENV_ENABLE_PROFILING, Some("true")),
+                (ENV_OUTPUT_DIR, Some("/tmp/rustfs-profiles")),
+            ],
+            || {
+                let keys = legacy_profiling_env_keys_present();
+                assert!(keys.contains(&ENV_ENABLE_PROFILING));
+                assert!(keys.contains(&ENV_OUTPUT_DIR));
+            },
+        );
+    }
+}