User-Events exporter - add validations, tests and nit cleanups (#195)

Author: cijothomas

.cspell.json

@@ -58,6 +58,7 @@
         "rustc",
         "Tescher",
         "tracepoint",
+        "tracepoints",
         "Tracepoints",
         "Zhongyang",
         "zipkin"

opentelemetry-user-events-logs/CHANGELOG.md

@@ -2,6 +2,15 @@
 
 ## vNext
 
+- Fixed contention in `event_enabled()` check and `export()` path, by caching the
+  EventSets, addressing
+  [159](https://github.com/open-telemetry/opentelemetry-rust-contrib/issues/159)
+- //TODO:(actually do this after upstream release) Shutdown now de-registers all the EventSets created
+- Added validation for the provider name in `with_user_event_exporter(provider_name)`.
+  The provider name must:
+  - Be less than 234 characters.
+  - Contain only ASCII letters, digits, and the underscore (`'_'`) character.
+
 ## v0.10.0
 
 - Removed provider group from being appended to the tracepoint name.

opentelemetry-user-events-logs/examples/basic-logs.rs

@@ -20,15 +20,16 @@ fn init_logger() -> SdkLoggerProvider {
 }
 
 fn main() {
-    // Example with tracing appender.
-    // Create a new tracing::Fmt layer to print the logs to stdout. It has a
-    // default filter of `info` level and above, and `debug` and above for logs
-    // from OpenTelemetry crates. The filter levels can be customized as needed.
+    // OpenTelemetry layer with a filter to ensure OTel's own logs are not fed back into
+    // the OpenTelemetry pipeline.
     let filter_otel = EnvFilter::new("info").add_directive("opentelemetry=off".parse().unwrap());
     let logger_provider = init_logger();
     let otel_layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
     let otel_layer = otel_layer.with_filter(filter_otel);
 
+    // Create a new tracing::Fmt layer to print the logs to stdout. It has a
+    // default filter of `info` level and above, and `debug` and above for logs
+    // from OpenTelemetry crates. The filter levels can be customized as needed.
     let filter_fmt = EnvFilter::new("info").add_directive("opentelemetry=debug".parse().unwrap());
     let fmt_layer = tracing_subscriber::fmt::layer().with_filter(filter_fmt);
 
@@ -37,8 +38,6 @@ fn main() {
         .with(fmt_layer)
         .init();
 
-    // event_id is passed as an attribute now, there is nothing in metadata where a
-    // numeric id can be stored.
     // run in a loop to ensure that tracepoints are not removed from kernel fs
 
     let running = Arc::new(AtomicBool::new(true));
@@ -50,6 +49,8 @@ fn main() {
     .expect("Error setting Ctrl-C handler");
 
     while running.load(Ordering::SeqCst) {
+        // event_id is passed as an attribute now, there is nothing in metadata where a
+        // numeric id can be stored.
         error!(
             name: "my-event-name",
             event_id = 20,

opentelemetry-user-events-logs/src/logs/exporter.rs

@@ -13,22 +13,38 @@ thread_local! { static EBW: RefCell<EventBuilder> = RefCell::new(EventBuilder::n
 /// UserEventsExporter is a log exporter that exports logs in EventHeader format to user_events tracepoint.
 pub(crate) struct UserEventsExporter {
     provider: Mutex<Provider>,
+    name: String,
     event_sets: Vec<Arc<EventSet>>,
 }
 
 const EVENT_ID: &str = "event_id";
 
 impl UserEventsExporter {
     /// Create instance of the exporter
-    pub(crate) fn new(provider_name: &str) -> Self {
+    pub(crate) fn new(provider_name: &str) -> Result<Self, String> {
+        // Validate provider_name
+        if provider_name.len() >= 234 {
+            return Err("Provider name must be less than 234 characters.".to_string());
+        }
+        if !provider_name
+            .chars()
+            .all(|c| c.is_ascii_alphanumeric() || c == '_')
+        {
+            return Err(
+                "Provider name must contain only ASCII letters, digits, and '_'.".to_string(),
+            );
+        }
+
         let mut eventheader_provider: Provider =
             Provider::new(provider_name, &Provider::new_options());
         let event_sets = Self::register_events(&mut eventheader_provider);
         otel_debug!(name: "UserEvents.Created", provider_name = provider_name);
-        UserEventsExporter {
+        let name = eventheader_provider.name().to_string();
+        Ok(UserEventsExporter {
             provider: Mutex::new(eventheader_provider),
+            name,
             event_sets,
-        }
+        })
     }
 
     fn register_events(
@@ -298,7 +314,7 @@ impl UserEventsExporter {
 
 impl Debug for UserEventsExporter {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str("user_events log exporter")
+        write!(f, "user_events log exporter (provider: {})", self.name)
     }
 }
 
@@ -335,3 +351,95 @@ impl opentelemetry_sdk::logs::LogExporter for UserEventsExporter {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    #[test]
+    fn exporter_debug() {
+        let exporter = UserEventsExporter::new("test_provider");
+        assert_eq!(
+            format!("{:?}", exporter.expect("Failed to create exporter")),
+            "user_events log exporter (provider: test_provider)"
+        );
+    }
+
+    #[test]
+    fn valid_provider_name() {
+        let valid_names = vec![
+            "ValidName",
+            "valid_name",
+            "Valid123",
+            "valid_123",
+            "_valid_name",
+            "VALID_NAME",
+        ];
+
+        for valid_name in valid_names {
+            let result = UserEventsExporter::new(valid_name);
+            assert!(
+                result.is_ok(),
+                "Expected '{}' to be valid, but it was rejected",
+                valid_name
+            );
+        }
+    }
+
+    #[test]
+    fn provider_name_too_long() {
+        let long_name = "a".repeat(234);
+        let result = UserEventsExporter::new(&long_name);
+        assert!(result.is_err());
+        assert_eq!(
+            result.err().unwrap(),
+            "Provider name must be less than 234 characters.".to_string()
+        );
+    }
+
+    #[test]
+    fn provider_name_contains_invalid_characters() {
+        // Define a vector of invalid provider names to test
+        let invalid_names = vec![
+            "Invalid Name",  // space
+            "Invalid:Name",  // colon
+            "Invalid\0Name", // null character
+            "Invalid-Name",  // hyphen
+            "InvalidName!",  // exclamation mark
+            "InvalidName@",  // at symbol
+            "Invalid+Name",  // plus
+            "Invalid&Name",  // ampersand
+            "Invalid#Name",  // hash
+            "Invalid%Name",  // percent
+            "Invalid/Name",  // slash
+            "Invalid\\Name", // backslash
+            "Invalid=Name",  // equals
+            "Invalid?Name",  // question mark
+            "Invalid;Name",  // semicolon
+            "Invalid,Name",  // comma
+        ];
+
+        // Expected error message
+        let expected_error =
+            "Provider name must contain only ASCII letters, digits, and '_'.".to_string();
+
+        // Test each invalid name
+        for invalid_name in invalid_names {
+            let result = UserEventsExporter::new(invalid_name);
+
+            // Assert that the result is an error
+            assert!(
+                result.is_err(),
+                "Expected '{}' to be invalid, but it was accepted",
+                invalid_name
+            );
+
+            // Assert that the error message is as expected
+            assert_eq!(
+                result.err().unwrap(),
+                expected_error,
+                "Wrong error message for invalid name: '{}'",
+                invalid_name
+            );
+        }
+    }
+}

opentelemetry-user-events-logs/src/logs/mod.rs

@@ -1,4 +1,5 @@
 use exporter::UserEventsExporter;
+use opentelemetry::otel_warn;
 use opentelemetry_sdk::logs::LoggerProviderBuilder;
 use reentrant_logprocessor::ReentrantLogProcessor;
 
@@ -9,24 +10,42 @@ mod reentrant_logprocessor;
 pub trait UserEventsLoggerProviderBuilderExt {
     /// Adds a user event exporter to the logger provider builder with the given provider name.
     ///
+    /// The provider name must:
+    /// - Be less than 234 characters.
+    /// - Contain only ASCII letters, digits, and the underscore (`'_'`) character.
+    /// - Be short, human-readable, and unique enough to avoid conflicts with other provider names.
+    /// - Typically include a company name and a component name, e.g., "MyCompany_MyComponent".
+    ///
+    /// If an invalid provider name is provided, this method will not add the exporter to the builder.
+    ///
     /// Tracepoint names are generated by combining the provider name, event
     /// level and keyword (currently hardcoded to `1`) in the following format:
     /// `ProviderName + '_' + 'L' + EventLevel + 'K' + EventKeyword`
     ///
     /// For example, if "myprovider" is the provider name, the following tracepoint names are created:
-    /// - `myprovider_L4K1`
     /// - `myprovider_L5K1`
+    /// - `myprovider_L4K1`
     /// - `myprovider_L3K1`
     /// - `myprovider_L2K1`
     /// - `myprovider_L1K1`
     ///
+    /// perf tool can be used to record events from the tracepoints.
+    /// For example the following will capture level 2 (Error) and 3(Warning) events:
+    /// perf record -e user_events:myprovider_L2K1,user_events:myprovider_L3K1
     fn with_user_event_exporter(self, provider_name: &str) -> Self;
 }
 
 impl UserEventsLoggerProviderBuilderExt for LoggerProviderBuilder {
     fn with_user_event_exporter(self, provider_name: &str) -> Self {
-        let exporter = UserEventsExporter::new(provider_name);
-        let reenterant_processor = ReentrantLogProcessor::new(exporter);
-        self.with_log_processor(reenterant_processor)
+        match UserEventsExporter::new(provider_name) {
+            Ok(exporter) => {
+                let reenterant_processor = ReentrantLogProcessor::new(exporter);
+                self.with_log_processor(reenterant_processor)
+            }
+            Err(e) => {
+                otel_warn!(name: "User_Events.Exporter.CreationFailed", reason = e);
+                self
+            }
+        }
     }
 }

opentelemetry-user-events-logs/src/logs/reentrant_logprocessor.rs

@@ -36,6 +36,8 @@ impl<T: LogExporter> opentelemetry_sdk::logs::LogProcessor for ReentrantLogProce
 
     // Nothing to shutdown
     fn shutdown(&self) -> OTelSdkResult {
+        // TODO: Actually invoke shutdown on the exporter
+        // This cannot be done today as it requires mutable reference to exporter.
         Ok(())
     }
 

stress/Cargo.toml

@@ -4,11 +4,6 @@ version = "0.1.0"
 edition = "2021"
 publish = false
 
-[[bin]]
-name = "eventheader"
-path = "src/eventheader.rs"
-doc = false
-
 [[bin]]
 name = "user_events"
 path = "src/user_events.rs"
@@ -21,11 +16,9 @@ doc = false
 
 [dependencies]
 ctrlc = "3.2.5"
-lazy_static = "1.4.0"
 num_cpus = "1.15.0"
 num-format = "0.4.4"
 sysinfo = { version = "0.32", optional = true }
-eventheader_dynamic = "0.4.0"
 
 opentelemetry-appender-tracing = { workspace = true, features= ["spec_unstable_logs_enabled"] }
 opentelemetry_sdk = { workspace = true, features = ["logs", "spec_unstable_logs_enabled"] }

stress/src/etw_logs.rs

@@ -43,23 +43,18 @@ fn init_logger() -> SdkLoggerProvider {
         .build()
 }
 
-// Function that performs the logging task
-fn log_event_task() {
-    info!(
-        name = "my-event-name",
-        event_id = 20,
-        user_name = "otel user",
-        user_email = "otel@opentelemetry.io"
-    );
-}
-
 fn main() {
     let logger_provider = init_logger();
     let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
     tracing_subscriber::registry().with(layer).init();
 
     throughput::test_throughput(|| {
-        log_event_task();
+        info!(
+            name : "my-event-name",
+            event_id = 20,
+            user_name = "otel user",
+            user_email = "otel@opentelemetry.io"
+        );
     });
 
     println!("Stress test completed.");

stress/src/eventheader.rs

@@ -39,16 +39,6 @@ fn init_logger() -> SdkLoggerProvider {
         .build()
 }
 
-// Function that performs the logging task
-fn log_event_task() {
-    error!(
-        name = "my-event-name",
-        event_id = 20,
-        user_name = "otel user",
-        user_email = "otel@opentelemetry.io"
-    );
-}
-
 fn main() {
     // Initialize the logger
     let logger_provider = init_logger();
@@ -60,7 +50,12 @@ fn main() {
     // Use the provided stress test framework
     println!("Starting stress test for UserEventsExporter...");
     throughput::test_throughput(|| {
-        log_event_task();
+        error!(
+            name : "my-event-name",
+            event_id = 20,
+            user_name = "otel user",
+            user_email = "otel@opentelemetry.io"
+        );
     });
     println!("Stress test completed.");
 }