From 1385dc201b01d9fdefdc86fffad8badf92edd938 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Wed, 6 Aug 2025 10:48:43 +0400 Subject: [PATCH 01/13] feat(user-events): auto-enable/disable listener in benchmark --- .../benches/logs.rs | 212 +++++++++++++++++- 1 file changed, 206 insertions(+), 6 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index d9217eb64..66e5caf76 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -5,21 +5,26 @@ Hardware: Apple M4 Pro Total Number of Cores: 10 (Inside multipass vm running Ubuntu 22.04) - // When no listener + + // When no listener (automatic disable via RAII guard) | Test | Average time| |-----------------------------|-------------| - | User_Event_4_Attributes | 8 ns | - | User_Event_6_Attributes | 8 ns | + | User_Event_4_Attributes | 19.2 ns | + | User_Event_6_Attributes | 19.6 ns | + | User_Event_4_Attributes_EventName_Custom | 20.2 ns | + | User_Event_4_Attributes_EventName_FromLogRecord | 20.6 ns | - // When listener is enabled - // Run below to enable + // When listener is enabled (automatic enable via RAII guard) + // The benchmark now automatically enables/disables the listener + // using the commands below internally: // echo 1 | sudo tee /sys/kernel/debug/tracing/events/user_events/myprovider_L2K1/enable - // Run below to disable // echo 0 | sudo tee /sys/kernel/debug/tracing/events/user_events/myprovider_L2K1/enable | Test | Average time| |-----------------------------|-------------| | User_Event_4_Attributes | 530 ns | | User_Event_6_Attributes | 586 ns | + | User_Event_4_Attributes_EventName_Custom | 590 ns | + | User_Event_4_Attributes_EventName_FromLogRecord | 595 ns | */ // running the following from the current directory @@ -35,6 +40,129 @@ use opentelemetry_user_events_logs::Processor; use tracing::error; use tracing_subscriber::prelude::*; use tracing_subscriber::Registry; +use std::process::Command; + +/// RAII guard for enabling/disabling user events listener +/// +/// This guard automatically enables the user events listener when created and +/// disables it when dropped, ensuring proper cleanup even if the benchmark +/// panics or exits early. +/// +/// The guard tracks whether the listener was already enabled before it was +/// created, and only disables it on drop if it wasn't already enabled. +/// This prevents interfering with other tests or processes that might have +/// enabled the listener. +struct UserEventsListenerGuard { + provider_name: String, + was_enabled: bool, +} + +impl UserEventsListenerGuard { + /// Enable the user events listener for the given provider + /// + /// This method: + /// 1. Checks if the listener is already enabled + /// 2. Enables it if it's not already enabled + /// 3. Returns a guard that will disable the listener on drop (if it wasn't already enabled) + /// + /// # Arguments + /// * `provider_name` - The name of the provider to enable/disable + /// + /// # Returns + /// * `Ok(Self)` - A guard that will disable the listener on drop + /// * `Err(String)` - Error message if enabling failed + fn enable(provider_name: &str) -> Result { + let enable_path = format!("/sys/kernel/debug/tracing/events/user_events/{}_L2K1/enable", provider_name); + + // Check if already enabled + let check_output = Command::new("sudo") + .arg("cat") + .arg(&enable_path) + .output() + .map_err(|e| format!("Failed to check listener status: {e}"))?; + + let was_enabled = check_output.status.success() && + String::from_utf8_lossy(&check_output.stdout).trim() == "1"; + + // Enable the listener + let enable_output = Command::new("sudo") + .arg("sh") + .arg("-c") + .arg(format!("echo 1 | sudo tee {}", enable_path)) + .output() + .map_err(|e| format!("Failed to enable listener: {e}"))?; + + if !enable_output.status.success() { + return Err(format!( + "Failed to enable listener. Error: {}", + String::from_utf8_lossy(&enable_output.stderr) + )); + } + + println!("User events listener enabled for provider: {}", provider_name); + + Ok(UserEventsListenerGuard { + provider_name: provider_name.to_string(), + was_enabled, + }) + } + + /// Check if user events are available on the system + /// + /// This method checks if the user_events subsystem is available by + /// reading from `/sys/kernel/tracing/user_events_status`. + /// + /// # Returns + /// * `Ok(String)` - The status content if user events are available + /// * `Err(String)` - Error message if user events are not available + fn check_user_events_available() -> Result { + let output = Command::new("sudo") + .arg("cat") + .arg("/sys/kernel/tracing/user_events_status") + .output() + .map_err(|e| format!("Failed to execute command: {e}"))?; + + if output.status.success() { + let status = String::from_utf8_lossy(&output.stdout); + Ok(status.to_string()) + } else { + Err(format!( + "Command executed with failing error code: {}", + String::from_utf8_lossy(&output.stderr) + )) + } + } +} + +impl Drop for UserEventsListenerGuard { + fn drop(&mut self) { + let disable_path = format!("/sys/kernel/debug/tracing/events/user_events/{}_L2K1/enable", self.provider_name); + + // Only disable if it wasn't already enabled + if !self.was_enabled { + let disable_output = Command::new("sudo") + .arg("sh") + .arg("-c") + .arg(format!("echo 0 | sudo tee {}", disable_path)) + .output(); + + match disable_output { + Ok(output) if output.status.success() => { + println!("User events listener disabled for provider: {}", self.provider_name); + } + Ok(output) => { + eprintln!("Failed to disable listener. Error: {}", + String::from_utf8_lossy(&output.stderr)); + } + Err(e) => { + eprintln!("Failed to disable listener: {}", e); + } + } + } else { + println!("User events listener was already enabled, leaving enabled for provider: {}", self.provider_name); + } + } +} #[cfg(feature = "experimental_eventname_callback")] struct EventNameFromLogRecordEventName; @@ -97,6 +225,24 @@ where } fn benchmark_4_attributes(c: &mut Criterion) { + // Check if user events are available + if let Err(e) = UserEventsListenerGuard::check_user_events_available() { + eprintln!("Warning: User events not available: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + } + + // Enable listener with RAII guard + let _guard = UserEventsListenerGuard::enable("myprovider") + .unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard { + provider_name: "dummy".to_string(), + was_enabled: true, + } + }); + let provider = setup_provider_default(); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -119,6 +265,24 @@ fn benchmark_4_attributes(c: &mut Criterion) { #[cfg(feature = "experimental_eventname_callback")] fn benchmark_4_attributes_event_name_custom(c: &mut Criterion) { + // Check if user events are available + if let Err(e) = UserEventsListenerGuard::check_user_events_available() { + eprintln!("Warning: User events not available: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + } + + // Enable listener with RAII guard + let _guard = UserEventsListenerGuard::enable("myprovider") + .unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard { + provider_name: "dummy".to_string(), + was_enabled: true, + } + }); + let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -141,6 +305,24 @@ fn benchmark_4_attributes_event_name_custom(c: &mut Criterion) { #[cfg(feature = "experimental_eventname_callback")] fn benchmark_4_attributes_event_name_from_log_record(c: &mut Criterion) { + // Check if user events are available + if let Err(e) = UserEventsListenerGuard::check_user_events_available() { + eprintln!("Warning: User events not available: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + } + + // Enable listener with RAII guard + let _guard = UserEventsListenerGuard::enable("myprovider") + .unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard { + provider_name: "dummy".to_string(), + was_enabled: true, + } + }); + let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -162,6 +344,24 @@ fn benchmark_4_attributes_event_name_from_log_record(c: &mut Criterion) { } fn benchmark_6_attributes(c: &mut Criterion) { + // Check if user events are available + if let Err(e) = UserEventsListenerGuard::check_user_events_available() { + eprintln!("Warning: User events not available: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + } + + // Enable listener with RAII guard + let _guard = UserEventsListenerGuard::enable("myprovider") + .unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard { + provider_name: "dummy".to_string(), + was_enabled: true, + } + }); + let provider = setup_provider_default(); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); From 37e5c1ff8af8c2ddaafc53e75359d0e2cb2c97d4 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Wed, 6 Aug 2025 10:58:12 +0400 Subject: [PATCH 02/13] fix(user-events-logs): enable required features by default to fix code analysis warnings --- opentelemetry-user-events-logs/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-user-events-logs/Cargo.toml b/opentelemetry-user-events-logs/Cargo.toml index cef983e0e..59158dfa8 100644 --- a/opentelemetry-user-events-logs/Cargo.toml +++ b/opentelemetry-user-events-logs/Cargo.toml @@ -33,7 +33,7 @@ serde_json = "1.0.140" spec_unstable_logs_enabled = ["opentelemetry/spec_unstable_logs_enabled", "opentelemetry_sdk/spec_unstable_logs_enabled", "opentelemetry-appender-tracing/spec_unstable_logs_enabled"] internal-logs = ["tracing", "opentelemetry/internal-logs", "opentelemetry_sdk/internal-logs"] experimental_eventname_callback = [] -default = ["internal-logs"] +default = ["internal-logs", "spec_unstable_logs_enabled", "experimental_eventname_callback"] [[bench]] From 89f98a07fb104b1d995831e143be3359271637aa Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Wed, 6 Aug 2025 11:02:14 +0400 Subject: [PATCH 03/13] feat(user-events): added CHANGELOG.md --- opentelemetry-user-events-logs/CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/opentelemetry-user-events-logs/CHANGELOG.md b/opentelemetry-user-events-logs/CHANGELOG.md index 5056443b7..0416e40ec 100644 --- a/opentelemetry-user-events-logs/CHANGELOG.md +++ b/opentelemetry-user-events-logs/CHANGELOG.md @@ -2,6 +2,15 @@ ## vNext +### Changed + +- Improved benchmark reliability by adding automatic enable/disable management for user events listener + - Added RAII guard (`UserEventsListenerGuard`) for automatic listener management + - Benchmarks now automatically enable listener when tests start and disable when complete + - Added graceful fallback for systems without user events support + - Updated benchmark results with current performance data + - Enabled required features by default to fix code analysis warnings + ## v0.14.0 Released 2025-July-24 From 61701e1337226d0e4cb5f6ad0bd1fffdffddded4 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Tue, 12 Aug 2025 16:49:18 +0400 Subject: [PATCH 04/13] feat(user-events): codereview fixes --- opentelemetry-user-events-logs/CHANGELOG.md | 8 -- opentelemetry-user-events-logs/Cargo.toml | 2 +- .../benches/logs.rs | 114 ++++++++++-------- 3 files changed, 66 insertions(+), 58 deletions(-) diff --git a/opentelemetry-user-events-logs/CHANGELOG.md b/opentelemetry-user-events-logs/CHANGELOG.md index 0416e40ec..9e69e9a44 100644 --- a/opentelemetry-user-events-logs/CHANGELOG.md +++ b/opentelemetry-user-events-logs/CHANGELOG.md @@ -2,14 +2,6 @@ ## vNext -### Changed - -- Improved benchmark reliability by adding automatic enable/disable management for user events listener - - Added RAII guard (`UserEventsListenerGuard`) for automatic listener management - - Benchmarks now automatically enable listener when tests start and disable when complete - - Added graceful fallback for systems without user events support - - Updated benchmark results with current performance data - - Enabled required features by default to fix code analysis warnings ## v0.14.0 diff --git a/opentelemetry-user-events-logs/Cargo.toml b/opentelemetry-user-events-logs/Cargo.toml index 59158dfa8..cef983e0e 100644 --- a/opentelemetry-user-events-logs/Cargo.toml +++ b/opentelemetry-user-events-logs/Cargo.toml @@ -33,7 +33,7 @@ serde_json = "1.0.140" spec_unstable_logs_enabled = ["opentelemetry/spec_unstable_logs_enabled", "opentelemetry_sdk/spec_unstable_logs_enabled", "opentelemetry-appender-tracing/spec_unstable_logs_enabled"] internal-logs = ["tracing", "opentelemetry/internal-logs", "opentelemetry_sdk/internal-logs"] experimental_eventname_callback = [] -default = ["internal-logs", "spec_unstable_logs_enabled", "experimental_eventname_callback"] +default = ["internal-logs"] [[bench]] diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index 66e5caf76..fbc6e6528 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -42,6 +42,11 @@ use tracing_subscriber::prelude::*; use tracing_subscriber::Registry; use std::process::Command; +const PROVIDER_NAME: &str = "myprovider"; +/// Suffix used by the kernel user_events provider naming in these benchmarks. +/// Documented to avoid magic strings in path construction. +const USER_EVENTS_PROVIDER_SUFFIX: &str = "_L2K1"; + /// RAII guard for enabling/disabling user events listener /// /// This guard automatically enables the user events listener when created and @@ -72,7 +77,10 @@ impl UserEventsListenerGuard { /// * `Ok(Self)` - A guard that will disable the listener on drop /// * `Err(String)` - Error message if enabling failed fn enable(provider_name: &str) -> Result { - let enable_path = format!("/sys/kernel/debug/tracing/events/user_events/{}_L2K1/enable", provider_name); + let enable_path = format!( + "/sys/kernel/debug/tracing/events/user_events/{}{}//enable", + provider_name, USER_EVENTS_PROVIDER_SUFFIX + ).replacen("//enable", "/enable", 1); // Check if already enabled let check_output = Command::new("sudo") @@ -84,12 +92,19 @@ impl UserEventsListenerGuard { let was_enabled = check_output.status.success() && String::from_utf8_lossy(&check_output.stdout).trim() == "1"; - // Enable the listener - let enable_output = Command::new("sudo") - .arg("sh") - .arg("-c") - .arg(format!("echo 1 | sudo tee {}", enable_path)) - .output() + // Enable the listener using a safe approach (no shell interpretation) + let mut enable_cmd = Command::new("sudo"); + enable_cmd.arg("tee").arg(&enable_path); + enable_cmd.stdin(std::process::Stdio::piped()); + let enable_output = enable_cmd + .spawn() + .and_then(|mut child| { + if let Some(stdin) = child.stdin.as_mut() { + use std::io::Write; + stdin.write_all(b"1\n")?; + } + child.wait_with_output() + }) .map_err(|e| format!("Failed to enable listener: {e}"))?; if !enable_output.status.success() { @@ -132,21 +147,38 @@ impl UserEventsListenerGuard { )) } } + + /// Create a dummy guard that performs no action on drop + /// + /// This is used when enabling the listener fails; it ensures the + /// benchmark can proceed without attempting to disable anything later. + fn dummy_guard() -> Self { + UserEventsListenerGuard { + provider_name: "dummy".to_string(), + was_enabled: true, + } + } } impl Drop for UserEventsListenerGuard { fn drop(&mut self) { - let disable_path = format!("/sys/kernel/debug/tracing/events/user_events/{}_L2K1/enable", self.provider_name); + let disable_path = format!( + "/sys/kernel/debug/tracing/events/user_events/{}{}//enable", + self.provider_name, USER_EVENTS_PROVIDER_SUFFIX + ).replacen("//enable", "/enable", 1); // Only disable if it wasn't already enabled if !self.was_enabled { - let disable_output = Command::new("sudo") - .arg("sh") - .arg("-c") - .arg(format!("echo 0 | sudo tee {}", disable_path)) - .output(); - - match disable_output { + let mut disable_cmd = Command::new("sudo"); + disable_cmd.arg("tee").arg(&disable_path); + disable_cmd.stdin(std::process::Stdio::piped()); + match disable_cmd.spawn().and_then(|mut child| { + if let Some(stdin) = child.stdin.as_mut() { + use std::io::Write; + stdin.write_all(b"0\n")?; + } + child.wait_with_output() + }) { Ok(output) if output.status.success() => { println!("User events listener disabled for provider: {}", self.provider_name); } @@ -192,7 +224,7 @@ impl EventNameCallback for EventNameFromLogRecordCustom { } fn setup_provider_default() -> SdkLoggerProvider { - let user_event_processor = Processor::builder("myprovider").build().unwrap(); + let user_event_processor = Processor::builder(PROVIDER_NAME).build().unwrap(); SdkLoggerProvider::builder() .with_resource( @@ -209,7 +241,7 @@ fn setup_provider_with_callback(event_name_callback: C) -> SdkLoggerProvider where C: EventNameCallback + 'static, { - let user_event_processor = Processor::builder("myprovider") + let user_event_processor = Processor::builder(PROVIDER_NAME) .with_event_name_callback(event_name_callback) .build() .unwrap(); @@ -231,19 +263,15 @@ fn benchmark_4_attributes(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - // Enable listener with RAII guard - let _guard = UserEventsListenerGuard::enable("myprovider") + let provider = setup_provider_default(); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) .unwrap_or_else(|e| { eprintln!("Warning: Failed to enable listener: {}", e); eprintln!("Benchmarks will run without listener enabled"); // Return a dummy guard that does nothing on drop - UserEventsListenerGuard { - provider_name: "dummy".to_string(), - was_enabled: true, - } + UserEventsListenerGuard::dummy_guard() }); - - let provider = setup_provider_default(); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -271,19 +299,15 @@ fn benchmark_4_attributes_event_name_custom(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - // Enable listener with RAII guard - let _guard = UserEventsListenerGuard::enable("myprovider") + let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) .unwrap_or_else(|e| { eprintln!("Warning: Failed to enable listener: {}", e); eprintln!("Benchmarks will run without listener enabled"); // Return a dummy guard that does nothing on drop - UserEventsListenerGuard { - provider_name: "dummy".to_string(), - was_enabled: true, - } + UserEventsListenerGuard::dummy_guard() }); - - let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -311,19 +335,15 @@ fn benchmark_4_attributes_event_name_from_log_record(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - // Enable listener with RAII guard - let _guard = UserEventsListenerGuard::enable("myprovider") + let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) .unwrap_or_else(|e| { eprintln!("Warning: Failed to enable listener: {}", e); eprintln!("Benchmarks will run without listener enabled"); // Return a dummy guard that does nothing on drop - UserEventsListenerGuard { - provider_name: "dummy".to_string(), - was_enabled: true, - } + UserEventsListenerGuard::dummy_guard() }); - - let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -350,19 +370,15 @@ fn benchmark_6_attributes(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - // Enable listener with RAII guard - let _guard = UserEventsListenerGuard::enable("myprovider") + let provider = setup_provider_default(); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) .unwrap_or_else(|e| { eprintln!("Warning: Failed to enable listener: {}", e); eprintln!("Benchmarks will run without listener enabled"); // Return a dummy guard that does nothing on drop - UserEventsListenerGuard { - provider_name: "dummy".to_string(), - was_enabled: true, - } + UserEventsListenerGuard::dummy_guard() }); - - let provider = setup_provider_default(); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); From f1d13dec03ea5b1b5547a762544d39f661d94dbd Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Wed, 13 Aug 2025 15:38:25 +0400 Subject: [PATCH 05/13] feat(user-events): cargo fmt --- .../benches/logs.rs | 125 ++++++++++-------- 1 file changed, 67 insertions(+), 58 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index fbc6e6528..fd013d727 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -5,7 +5,7 @@ Hardware: Apple M4 Pro Total Number of Cores: 10 (Inside multipass vm running Ubuntu 22.04) - + // When no listener (automatic disable via RAII guard) | Test | Average time| |-----------------------------|-------------| @@ -37,10 +37,10 @@ use opentelemetry_sdk::Resource; #[cfg(feature = "experimental_eventname_callback")] use opentelemetry_user_events_logs::EventNameCallback; use opentelemetry_user_events_logs::Processor; +use std::process::Command; use tracing::error; use tracing_subscriber::prelude::*; use tracing_subscriber::Registry; -use std::process::Command; const PROVIDER_NAME: &str = "myprovider"; /// Suffix used by the kernel user_events provider naming in these benchmarks. @@ -48,11 +48,11 @@ const PROVIDER_NAME: &str = "myprovider"; const USER_EVENTS_PROVIDER_SUFFIX: &str = "_L2K1"; /// RAII guard for enabling/disabling user events listener -/// +/// /// This guard automatically enables the user events listener when created and /// disables it when dropped, ensuring proper cleanup even if the benchmark /// panics or exits early. -/// +/// /// The guard tracks whether the listener was already enabled before it was /// created, and only disables it on drop if it wasn't already enabled. /// This prevents interfering with other tests or processes that might have @@ -64,15 +64,15 @@ struct UserEventsListenerGuard { impl UserEventsListenerGuard { /// Enable the user events listener for the given provider - /// + /// /// This method: /// 1. Checks if the listener is already enabled /// 2. Enables it if it's not already enabled /// 3. Returns a guard that will disable the listener on drop (if it wasn't already enabled) - /// + /// /// # Arguments /// * `provider_name` - The name of the provider to enable/disable - /// + /// /// # Returns /// * `Ok(Self)` - A guard that will disable the listener on drop /// * `Err(String)` - Error message if enabling failed @@ -80,18 +80,19 @@ impl UserEventsListenerGuard { let enable_path = format!( "/sys/kernel/debug/tracing/events/user_events/{}{}//enable", provider_name, USER_EVENTS_PROVIDER_SUFFIX - ).replacen("//enable", "/enable", 1); - + ) + .replacen("//enable", "/enable", 1); + // Check if already enabled let check_output = Command::new("sudo") .arg("cat") .arg(&enable_path) .output() .map_err(|e| format!("Failed to check listener status: {e}"))?; - - let was_enabled = check_output.status.success() && - String::from_utf8_lossy(&check_output.stdout).trim() == "1"; - + + let was_enabled = check_output.status.success() + && String::from_utf8_lossy(&check_output.stdout).trim() == "1"; + // Enable the listener using a safe approach (no shell interpretation) let mut enable_cmd = Command::new("sudo"); enable_cmd.arg("tee").arg(&enable_path); @@ -106,27 +107,30 @@ impl UserEventsListenerGuard { child.wait_with_output() }) .map_err(|e| format!("Failed to enable listener: {e}"))?; - + if !enable_output.status.success() { return Err(format!( "Failed to enable listener. Error: {}", String::from_utf8_lossy(&enable_output.stderr) )); } - - println!("User events listener enabled for provider: {}", provider_name); - + + println!( + "User events listener enabled for provider: {}", + provider_name + ); + Ok(UserEventsListenerGuard { provider_name: provider_name.to_string(), was_enabled, }) } - + /// Check if user events are available on the system - /// + /// /// This method checks if the user_events subsystem is available by /// reading from `/sys/kernel/tracing/user_events_status`. - /// + /// /// # Returns /// * `Ok(String)` - The status content if user events are available /// * `Err(String)` - Error message if user events are not available @@ -165,8 +169,9 @@ impl Drop for UserEventsListenerGuard { let disable_path = format!( "/sys/kernel/debug/tracing/events/user_events/{}{}//enable", self.provider_name, USER_EVENTS_PROVIDER_SUFFIX - ).replacen("//enable", "/enable", 1); - + ) + .replacen("//enable", "/enable", 1); + // Only disable if it wasn't already enabled if !self.was_enabled { let mut disable_cmd = Command::new("sudo"); @@ -180,18 +185,26 @@ impl Drop for UserEventsListenerGuard { child.wait_with_output() }) { Ok(output) if output.status.success() => { - println!("User events listener disabled for provider: {}", self.provider_name); + println!( + "User events listener disabled for provider: {}", + self.provider_name + ); } Ok(output) => { - eprintln!("Failed to disable listener. Error: {}", - String::from_utf8_lossy(&output.stderr)); + eprintln!( + "Failed to disable listener. Error: {}", + String::from_utf8_lossy(&output.stderr) + ); } Err(e) => { eprintln!("Failed to disable listener: {}", e); } } } else { - println!("User events listener was already enabled, leaving enabled for provider: {}", self.provider_name); + println!( + "User events listener was already enabled, leaving enabled for provider: {}", + self.provider_name + ); } } } @@ -262,16 +275,15 @@ fn benchmark_4_attributes(c: &mut Criterion) { eprintln!("Warning: User events not available: {}", e); eprintln!("Benchmarks will run without listener enabled"); } - + let provider = setup_provider_default(); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) - .unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -298,16 +310,15 @@ fn benchmark_4_attributes_event_name_custom(c: &mut Criterion) { eprintln!("Warning: User events not available: {}", e); eprintln!("Benchmarks will run without listener enabled"); } - + let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) - .unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -334,16 +345,15 @@ fn benchmark_4_attributes_event_name_from_log_record(c: &mut Criterion) { eprintln!("Warning: User events not available: {}", e); eprintln!("Benchmarks will run without listener enabled"); } - + let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) - .unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -369,16 +379,15 @@ fn benchmark_6_attributes(c: &mut Criterion) { eprintln!("Warning: User events not available: {}", e); eprintln!("Benchmarks will run without listener enabled"); } - + let provider = setup_provider_default(); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME) - .unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); From 7e088faa1d5b7879126e930a4cb58b7f554d1f64 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 09:22:50 +0400 Subject: [PATCH 06/13] feat: benchmark both enabled and disabled user events listener states --- .../benches/logs.rs | 306 ++++++++++++------ 1 file changed, 212 insertions(+), 94 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index fd013d727..b7682ee74 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -276,31 +276,60 @@ fn benchmark_4_attributes(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - let provider = setup_provider_default(); - // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); - let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); - let subscriber = Registry::default().with(ot_layer); - - tracing::subscriber::with_default(subscriber, || { - c.bench_function("User_Event_4_Attributes", |b| { - b.iter(|| { - error!( - name : "CheckoutFailed", - field1 = "field1", - field2 = "field2", - field3 = "field3", - field4 = "field4", - message = "Unable to process checkout." - ); + let mut group = c.benchmark_group("User_Event_4_Attributes"); + + // Benchmark with listener disabled + { + let provider = setup_provider_default(); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("disabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + message = "Unable to process checkout." + ); + }); + }); + }); + } + + // Benchmark with listener enabled + { + let provider = setup_provider_default(); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("enabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + message = "Unable to process checkout." + ); + }); }); }); - }); + } + + group.finish(); } #[cfg(feature = "experimental_eventname_callback")] @@ -311,31 +340,60 @@ fn benchmark_4_attributes_event_name_custom(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); - // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); - let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); - let subscriber = Registry::default().with(ot_layer); - - tracing::subscriber::with_default(subscriber, || { - c.bench_function("User_Event_4_Attributes_EventName_Custom", |b| { - b.iter(|| { - error!( - name : "CheckoutFailed", - field1 = "field1", - field2 = "field2", - field3 = "field3", - field4 = "field4", - message = "Unable to process checkout." - ); + let mut group = c.benchmark_group("User_Event_4_Attributes_EventName_Custom"); + + // Benchmark with listener disabled + { + let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("disabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + message = "Unable to process checkout." + ); + }); + }); + }); + } + + // Benchmark with listener enabled + { + let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("enabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + message = "Unable to process checkout." + ); + }); }); }); - }); + } + + group.finish(); } #[cfg(feature = "experimental_eventname_callback")] @@ -346,31 +404,60 @@ fn benchmark_4_attributes_event_name_from_log_record(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); - // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); - let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); - let subscriber = Registry::default().with(ot_layer); - - tracing::subscriber::with_default(subscriber, || { - c.bench_function("User_Event_4_Attributes_EventName_FromLogRecord", |b| { - b.iter(|| { - error!( - name : "CheckoutFailed", - field1 = "field1", - field2 = "field2", - field3 = "field3", - field4 = "field4", - message = "Unable to process checkout." - ); + let mut group = c.benchmark_group("User_Event_4_Attributes_EventName_FromLogRecord"); + + // Benchmark with listener disabled + { + let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("disabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + message = "Unable to process checkout." + ); + }); + }); + }); + } + + // Benchmark with listener enabled + { + let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("enabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + message = "Unable to process checkout." + ); + }); }); }); - }); + } + + group.finish(); } fn benchmark_6_attributes(c: &mut Criterion) { @@ -380,33 +467,64 @@ fn benchmark_6_attributes(c: &mut Criterion) { eprintln!("Benchmarks will run without listener enabled"); } - let provider = setup_provider_default(); - // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); - let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); - let subscriber = Registry::default().with(ot_layer); - - tracing::subscriber::with_default(subscriber, || { - c.bench_function("User_Event_6_Attributes", |b| { - b.iter(|| { - error!( - name : "CheckoutFailed", - field1 = "field1", - field2 = "field2", - field3 = "field3", - field4 = "field4", - field5 = "field5", - field6 = "field6", - message = "Unable to process checkout." - ); + let mut group = c.benchmark_group("User_Event_6_Attributes"); + + // Benchmark with listener disabled + { + let provider = setup_provider_default(); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("disabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + field5 = "field5", + field6 = "field6", + message = "Unable to process checkout." + ); + }); + }); + }); + } + + // Benchmark with listener enabled + { + let provider = setup_provider_default(); + // Enable listener with RAII guard (after provider is built so tracepoints exist) + let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }); + let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); + let subscriber = Registry::default().with(ot_layer); + + tracing::subscriber::with_default(subscriber, || { + group.bench_function("enabled", |b| { + b.iter(|| { + error!( + name : "CheckoutFailed", + field1 = "field1", + field2 = "field2", + field3 = "field3", + field4 = "field4", + field5 = "field5", + field6 = "field6", + message = "Unable to process checkout." + ); + }); }); }); - }); + } + + group.finish(); } fn criterion_benchmark(c: &mut Criterion) { From e20ad335d410b7124671ddbad9337a3b1993a4d9 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 14:46:42 +0400 Subject: [PATCH 07/13] fix: remove unnedded backslashes --- opentelemetry-user-events-logs/benches/logs.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index b7682ee74..f95c71f4a 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -78,10 +78,9 @@ impl UserEventsListenerGuard { /// * `Err(String)` - Error message if enabling failed fn enable(provider_name: &str) -> Result { let enable_path = format!( - "/sys/kernel/debug/tracing/events/user_events/{}{}//enable", + "/sys/kernel/debug/tracing/events/user_events/{}{}/enable", provider_name, USER_EVENTS_PROVIDER_SUFFIX - ) - .replacen("//enable", "/enable", 1); + ); // Check if already enabled let check_output = Command::new("sudo") @@ -167,10 +166,9 @@ impl UserEventsListenerGuard { impl Drop for UserEventsListenerGuard { fn drop(&mut self) { let disable_path = format!( - "/sys/kernel/debug/tracing/events/user_events/{}{}//enable", + "/sys/kernel/debug/tracing/events/user_events/{}{}/enable", self.provider_name, USER_EVENTS_PROVIDER_SUFFIX - ) - .replacen("//enable", "/enable", 1); + ); // Only disable if it wasn't already enabled if !self.was_enabled { From a00b2293c8365c9f5eacd42b43d726d92499c537 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 14:56:05 +0400 Subject: [PATCH 08/13] fix: remove sudo --- .../benches/logs.rs | 112 ++++++++---------- 1 file changed, 50 insertions(+), 62 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index f95c71f4a..937e3b15b 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -37,7 +37,8 @@ use opentelemetry_sdk::Resource; #[cfg(feature = "experimental_eventname_callback")] use opentelemetry_user_events_logs::EventNameCallback; use opentelemetry_user_events_logs::Processor; -use std::process::Command; +use std::fs; +use std::io; use tracing::error; use tracing_subscriber::prelude::*; use tracing_subscriber::Registry; @@ -82,36 +83,31 @@ impl UserEventsListenerGuard { provider_name, USER_EVENTS_PROVIDER_SUFFIX ); - // Check if already enabled - let check_output = Command::new("sudo") - .arg("cat") - .arg(&enable_path) - .output() - .map_err(|e| format!("Failed to check listener status: {e}"))?; - - let was_enabled = check_output.status.success() - && String::from_utf8_lossy(&check_output.stdout).trim() == "1"; - - // Enable the listener using a safe approach (no shell interpretation) - let mut enable_cmd = Command::new("sudo"); - enable_cmd.arg("tee").arg(&enable_path); - enable_cmd.stdin(std::process::Stdio::piped()); - let enable_output = enable_cmd - .spawn() - .and_then(|mut child| { - if let Some(stdin) = child.stdin.as_mut() { - use std::io::Write; - stdin.write_all(b"1\n")?; + // Check if already enabled by reading the file directly + let was_enabled = match fs::read_to_string(&enable_path) { + Ok(contents) => contents.trim() == "1", + Err(e) => { + if e.kind() == io::ErrorKind::PermissionDenied { + return Err(format!( + "Insufficient permissions to read '{}'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", + enable_path, e + )); + } else { + return Err(format!("Failed to check listener status: {}", e)); } - child.wait_with_output() - }) - .map_err(|e| format!("Failed to enable listener: {e}"))?; - - if !enable_output.status.success() { - return Err(format!( - "Failed to enable listener. Error: {}", - String::from_utf8_lossy(&enable_output.stderr) - )); + } + }; + + // Enable the listener by writing "1" to the enable file + if let Err(e) = fs::write(&enable_path, b"1\n") { + if e.kind() == io::ErrorKind::PermissionDenied { + return Err(format!( + "Insufficient permissions to write to '{}'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", + enable_path, e + )); + } else { + return Err(format!("Failed to enable listener: {}", e)); + } } println!( @@ -134,20 +130,20 @@ impl UserEventsListenerGuard { /// * `Ok(String)` - The status content if user events are available /// * `Err(String)` - Error message if user events are not available fn check_user_events_available() -> Result { - let output = Command::new("sudo") - .arg("cat") - .arg("/sys/kernel/tracing/user_events_status") - .output() - .map_err(|e| format!("Failed to execute command: {e}"))?; - - if output.status.success() { - let status = String::from_utf8_lossy(&output.stdout); - Ok(status.to_string()) - } else { - Err(format!( - "Command executed with failing error code: {}", - String::from_utf8_lossy(&output.stderr) - )) + match fs::read_to_string("/sys/kernel/tracing/user_events_status") { + Ok(status) => Ok(status), + Err(e) => { + if e.kind() == io::ErrorKind::PermissionDenied { + Err(format!( + "Insufficient permissions to read '/sys/kernel/tracing/user_events_status'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", + e + )) + } else if e.kind() == io::ErrorKind::NotFound { + Err("User events subsystem not available on this system".to_string()) + } else { + Err(format!("Failed to check user events availability: {}", e)) + } + } } } @@ -172,30 +168,22 @@ impl Drop for UserEventsListenerGuard { // Only disable if it wasn't already enabled if !self.was_enabled { - let mut disable_cmd = Command::new("sudo"); - disable_cmd.arg("tee").arg(&disable_path); - disable_cmd.stdin(std::process::Stdio::piped()); - match disable_cmd.spawn().and_then(|mut child| { - if let Some(stdin) = child.stdin.as_mut() { - use std::io::Write; - stdin.write_all(b"0\n")?; - } - child.wait_with_output() - }) { - Ok(output) if output.status.success() => { + match fs::write(&disable_path, b"0\n") { + Ok(_) => { println!( "User events listener disabled for provider: {}", self.provider_name ); } - Ok(output) => { - eprintln!( - "Failed to disable listener. Error: {}", - String::from_utf8_lossy(&output.stderr) - ); - } Err(e) => { - eprintln!("Failed to disable listener: {}", e); + if e.kind() == io::ErrorKind::PermissionDenied { + eprintln!( + "Failed to disable listener due to insufficient permissions. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", + e + ); + } else { + eprintln!("Failed to disable listener: {}", e); + } } } } else { From 2e2c4792b8637be1344c0c3e5cb131d360d6d8a4 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 17:04:06 +0400 Subject: [PATCH 09/13] fix: codereview comments --- opentelemetry-user-events-logs/benches/logs.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index 937e3b15b..a0a6e15b7 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -47,6 +47,9 @@ const PROVIDER_NAME: &str = "myprovider"; /// Suffix used by the kernel user_events provider naming in these benchmarks. /// Documented to avoid magic strings in path construction. const USER_EVENTS_PROVIDER_SUFFIX: &str = "_L2K1"; +/// Provider name used for dummy guards that perform no operations. +/// This value is not used for actual provider operations, only for internal tracking. +const DUMMY_PROVIDER_NAME: &str = "dummy"; /// RAII guard for enabling/disabling user events listener /// @@ -58,6 +61,14 @@ const USER_EVENTS_PROVIDER_SUFFIX: &str = "_L2K1"; /// created, and only disables it on drop if it wasn't already enabled. /// This prevents interfering with other tests or processes that might have /// enabled the listener. +/// +/// # Fields +/// * `provider_name` - The name of the provider being managed +/// * `was_enabled` - Tracks whether the listener was already enabled before this guard was created. +/// This field is crucial for preventing the guard from disabling listeners that were enabled +/// by external processes or other parts of the system. When the guard is dropped, it only +/// disables the listener if `was_enabled` is false, ensuring that external processes that +/// may have enabled the listener for their own purposes are not disrupted. struct UserEventsListenerGuard { provider_name: String, was_enabled: bool, @@ -153,7 +164,7 @@ impl UserEventsListenerGuard { /// benchmark can proceed without attempting to disable anything later. fn dummy_guard() -> Self { UserEventsListenerGuard { - provider_name: "dummy".to_string(), + provider_name: DUMMY_PROVIDER_NAME.to_string(), was_enabled: true, } } From 950efdce0184c9526f02e8e63a0997263646e4be Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 17:10:00 +0400 Subject: [PATCH 10/13] fix: remove dummy function call --- .../benches/logs.rs | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index a0a6e15b7..aeb79324e 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -206,6 +206,27 @@ impl Drop for UserEventsListenerGuard { } } +/// Helper function to enable user events listener with fallback to dummy guard +/// +/// This function attempts to enable the user events listener for the given provider. +/// If enabling fails, it prints a warning message and returns a dummy guard that +/// performs no operations. This ensures benchmarks can proceed even when the +/// listener cannot be enabled. +/// +/// # Arguments +/// * `provider_name` - The name of the provider to enable +/// +/// # Returns +/// * `UserEventsListenerGuard` - Either a real guard or a dummy guard +fn enable_listener_with_fallback(provider_name: &str) -> UserEventsListenerGuard { + UserEventsListenerGuard::enable(provider_name).unwrap_or_else(|e| { + eprintln!("Warning: Failed to enable listener: {}", e); + eprintln!("Benchmarks will run without listener enabled"); + // Return a dummy guard that does nothing on drop + UserEventsListenerGuard::dummy_guard() + }) +} + #[cfg(feature = "experimental_eventname_callback")] struct EventNameFromLogRecordEventName; @@ -301,12 +322,7 @@ fn benchmark_4_attributes(c: &mut Criterion) { { let provider = setup_provider_default(); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = enable_listener_with_fallback(PROVIDER_NAME); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -365,12 +381,7 @@ fn benchmark_4_attributes_event_name_custom(c: &mut Criterion) { { let provider = setup_provider_with_callback(EventNameFromLogRecordCustom); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = enable_listener_with_fallback(PROVIDER_NAME); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -429,12 +440,7 @@ fn benchmark_4_attributes_event_name_from_log_record(c: &mut Criterion) { { let provider = setup_provider_with_callback(EventNameFromLogRecordEventName); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = enable_listener_with_fallback(PROVIDER_NAME); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); @@ -494,12 +500,7 @@ fn benchmark_6_attributes(c: &mut Criterion) { { let provider = setup_provider_default(); // Enable listener with RAII guard (after provider is built so tracepoints exist) - let _guard = UserEventsListenerGuard::enable(PROVIDER_NAME).unwrap_or_else(|e| { - eprintln!("Warning: Failed to enable listener: {}", e); - eprintln!("Benchmarks will run without listener enabled"); - // Return a dummy guard that does nothing on drop - UserEventsListenerGuard::dummy_guard() - }); + let _guard = enable_listener_with_fallback(PROVIDER_NAME); let ot_layer = tracing_layer::OpenTelemetryTracingBridge::new(&provider); let subscriber = Registry::default().with(ot_layer); From ec1b4bf5c003dd520fe58ca63b6b26ed2a00494b Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 17:17:12 +0400 Subject: [PATCH 11/13] fix: optimisation, docs --- .../benches/logs.rs | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index aeb79324e..73f54ecf6 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -38,7 +38,7 @@ use opentelemetry_sdk::Resource; use opentelemetry_user_events_logs::EventNameCallback; use opentelemetry_user_events_logs::Processor; use std::fs; -use std::io; +use std::io::{self, Read}; use tracing::error; use tracing_subscriber::prelude::*; use tracing_subscriber::Registry; @@ -63,7 +63,10 @@ const DUMMY_PROVIDER_NAME: &str = "dummy"; /// enabled the listener. /// /// # Fields -/// * `provider_name` - The name of the provider being managed +/// * `provider_name` - The name of the kernel user events provider being managed by this guard. +/// This is used to construct sysfs paths and identify which provider's listener should be enabled or disabled. +/// It is essential for ensuring that the guard operates on the correct provider, especially when multiple +/// providers may exist or when running benchmarks that interact with different user events sources. /// * `was_enabled` - Tracks whether the listener was already enabled before this guard was created. /// This field is crucial for preventing the guard from disabling listeners that were enabled /// by external processes or other parts of the system. When the guard is dropped, it only @@ -94,9 +97,18 @@ impl UserEventsListenerGuard { provider_name, USER_EVENTS_PROVIDER_SUFFIX ); - // Check if already enabled by reading the file directly - let was_enabled = match fs::read_to_string(&enable_path) { - Ok(contents) => contents.trim() == "1", + // Check if already enabled by reading only the first byte of the file + let was_enabled = match fs::File::open(&enable_path) { + Ok(mut file) => { + let mut buf = [0u8; 1]; + match file.read(&mut buf) { + Ok(0) => false, // empty file, treat as disabled + Ok(_) => buf[0] == b'1', + Err(e) => { + return Err(format!("Failed to read listener status: {}", e)); + } + } + } Err(e) => { if e.kind() == io::ErrorKind::PermissionDenied { return Err(format!( @@ -110,7 +122,8 @@ impl UserEventsListenerGuard { }; // Enable the listener by writing "1" to the enable file - if let Err(e) = fs::write(&enable_path, b"1\n") { + // Note: No newline needed since we only read the first byte when checking status + if let Err(e) = fs::write(&enable_path, b"1") { if e.kind() == io::ErrorKind::PermissionDenied { return Err(format!( "Insufficient permissions to write to '{}'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", @@ -179,9 +192,9 @@ impl Drop for UserEventsListenerGuard { // Only disable if it wasn't already enabled if !self.was_enabled { - match fs::write(&disable_path, b"0\n") { + match fs::write(&disable_path, b"0") { Ok(_) => { - println!( + eprintln!( "User events listener disabled for provider: {}", self.provider_name ); @@ -198,7 +211,7 @@ impl Drop for UserEventsListenerGuard { } } } else { - println!( + eprintln!( "User events listener was already enabled, leaving enabled for provider: {}", self.provider_name ); From 2c72726813ed12569a2f04c7d362611188a878cc Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 17:25:19 +0400 Subject: [PATCH 12/13] fix: codereview fixes --- .../benches/logs.rs | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index 73f54ecf6..de98fac1a 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -38,7 +38,7 @@ use opentelemetry_sdk::Resource; use opentelemetry_user_events_logs::EventNameCallback; use opentelemetry_user_events_logs::Processor; use std::fs; -use std::io::{self, Read}; +use std::io; use tracing::error; use tracing_subscriber::prelude::*; use tracing_subscriber::Registry; @@ -72,9 +72,14 @@ const DUMMY_PROVIDER_NAME: &str = "dummy"; /// by external processes or other parts of the system. When the guard is dropped, it only /// disables the listener if `was_enabled` is false, ensuring that external processes that /// may have enabled the listener for their own purposes are not disrupted. +/// * `is_dummy` - Indicates whether this is a dummy guard that performs no operations. +/// Dummy guards are created when enabling the listener fails, ensuring benchmarks can proceed +/// without attempting to disable anything later. When `is_dummy` is true, the drop implementation +/// performs no cleanup operations. struct UserEventsListenerGuard { provider_name: String, was_enabled: bool, + is_dummy: bool, } impl UserEventsListenerGuard { @@ -97,17 +102,11 @@ impl UserEventsListenerGuard { provider_name, USER_EVENTS_PROVIDER_SUFFIX ); - // Check if already enabled by reading only the first byte of the file - let was_enabled = match fs::File::open(&enable_path) { - Ok(mut file) => { - let mut buf = [0u8; 1]; - match file.read(&mut buf) { - Ok(0) => false, // empty file, treat as disabled - Ok(_) => buf[0] == b'1', - Err(e) => { - return Err(format!("Failed to read listener status: {}", e)); - } - } + // Check if already enabled by reading the entire file and checking if it starts with '1' + let was_enabled = match fs::read_to_string(&enable_path) { + Ok(contents) => { + let trimmed = contents.trim_start(); + trimmed.starts_with('1') } Err(e) => { if e.kind() == io::ErrorKind::PermissionDenied { @@ -121,9 +120,9 @@ impl UserEventsListenerGuard { } }; - // Enable the listener by writing "1" to the enable file - // Note: No newline needed since we only read the first byte when checking status - if let Err(e) = fs::write(&enable_path, b"1") { + // Enable the listener by writing "1\n" to the enable file + // Most sysfs/debugfs interfaces expect a newline character. + if let Err(e) = fs::write(&enable_path, b"1\n") { if e.kind() == io::ErrorKind::PermissionDenied { return Err(format!( "Insufficient permissions to write to '{}'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", @@ -142,6 +141,7 @@ impl UserEventsListenerGuard { Ok(UserEventsListenerGuard { provider_name: provider_name.to_string(), was_enabled, + is_dummy: false, }) } @@ -178,7 +178,8 @@ impl UserEventsListenerGuard { fn dummy_guard() -> Self { UserEventsListenerGuard { provider_name: DUMMY_PROVIDER_NAME.to_string(), - was_enabled: true, + was_enabled: false, // Not relevant for dummy guards + is_dummy: true, } } } @@ -190,9 +191,14 @@ impl Drop for UserEventsListenerGuard { self.provider_name, USER_EVENTS_PROVIDER_SUFFIX ); + // Skip cleanup for dummy guards + if self.is_dummy { + return; + } + // Only disable if it wasn't already enabled if !self.was_enabled { - match fs::write(&disable_path, b"0") { + match fs::write(&disable_path, b"0\n") { Ok(_) => { eprintln!( "User events listener disabled for provider: {}", From 871b6524e53c1f7d34616843fcc07c994e15ede3 Mon Sep 17 00:00:00 2001 From: Artem Ziborev Date: Thu, 14 Aug 2025 17:35:35 +0400 Subject: [PATCH 13/13] fix: copilot codereview fixes --- .../benches/logs.rs | 55 ++++++++++++------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/opentelemetry-user-events-logs/benches/logs.rs b/opentelemetry-user-events-logs/benches/logs.rs index de98fac1a..6c3e772d6 100644 --- a/opentelemetry-user-events-logs/benches/logs.rs +++ b/opentelemetry-user-events-logs/benches/logs.rs @@ -46,6 +46,16 @@ use tracing_subscriber::Registry; const PROVIDER_NAME: &str = "myprovider"; /// Suffix used by the kernel user_events provider naming in these benchmarks. /// Documented to avoid magic strings in path construction. +/// +/// The kernel appends a unique suffix (such as "_L2K1") to user events provider names +/// when registering them via the user_events interface. This suffix is generated by the +/// kernel to ensure uniqueness and to encode internal information about the provider. +/// In this benchmark, "_L2K1" is the suffix assigned by the kernel to the "myprovider" +/// provider. This value may differ across systems or kernel versions, and is required +/// when constructing sysfs paths for enabling/disabling the provider's listener, e.g.: +/// /sys/kernel/debug/tracing/events/user_events/myprovider_L2K1/enable +/// For more details, see the Linux kernel documentation on user_events: +/// https://docs.kernel.org/trace/user_events.html const USER_EVENTS_PROVIDER_SUFFIX: &str = "_L2K1"; /// Provider name used for dummy guards that perform no operations. /// This value is not used for actual provider operations, only for internal tracking. @@ -102,12 +112,9 @@ impl UserEventsListenerGuard { provider_name, USER_EVENTS_PROVIDER_SUFFIX ); - // Check if already enabled by reading the entire file and checking if it starts with '1' + // Check if already enabled by reading the entire file and checking if it equals "1" let was_enabled = match fs::read_to_string(&enable_path) { - Ok(contents) => { - let trimmed = contents.trim_start(); - trimmed.starts_with('1') - } + Ok(contents) => contents.trim() == "1", Err(e) => { if e.kind() == io::ErrorKind::PermissionDenied { return Err(format!( @@ -120,23 +127,33 @@ impl UserEventsListenerGuard { } }; - // Enable the listener by writing "1\n" to the enable file - // Most sysfs/debugfs interfaces expect a newline character. - if let Err(e) = fs::write(&enable_path, b"1\n") { - if e.kind() == io::ErrorKind::PermissionDenied { - return Err(format!( - "Insufficient permissions to write to '{}'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", - enable_path, e - )); - } else { - return Err(format!("Failed to enable listener: {}", e)); + // Only enable the listener if it's not already enabled + if !was_enabled { + // Enable the listener by writing "1\n" to the enable file + // Most sysfs/debugfs interfaces expect a newline character. + if let Err(e) = fs::write(&enable_path, b"1\n") { + if e.kind() == io::ErrorKind::PermissionDenied { + return Err(format!( + "Insufficient permissions to write to '{}'. Please run the benchmark as root or with appropriate capabilities (CAP_SYS_ADMIN). Error: {}", + enable_path, e + )); + } else { + return Err(format!("Failed to enable listener: {}", e)); + } } } - println!( - "User events listener enabled for provider: {}", - provider_name - ); + if was_enabled { + println!( + "User events listener was already enabled for provider: {}", + provider_name + ); + } else { + println!( + "User events listener enabled for provider: {}", + provider_name + ); + } Ok(UserEventsListenerGuard { provider_name: provider_name.to_string(),