reporting: reporter factory has less responsibility

Author: rizsotto

bear/src/intercept/reporter.rs

@@ -13,11 +13,10 @@
 //!
 //! # Usage
 //!
-//! Use `ReporterFactory::create()` to instantiate a reporter, or `ReporterFactory::create_as_ptr()`
-//! to obtain a raw pointer suitable for static/global usage. The reporter sends events to a remote
-//! collector, with the destination specified by the `KEY_DESTINATION` environment variable.
+//! Use `ReporterFactory::create()` to instantiate a reporter with a socket address, or
+//! `ReporterFactory::create_as_ptr()` to obtain a raw pointer suitable for static/global usage.
+//! The reporter sends events to a remote collector at the specified address.
 
-use crate::environment::KEY_DESTINATION;
 use crate::intercept::{Event, tcp};
 use std::net::SocketAddr;
 use std::sync::atomic::AtomicPtr;
@@ -48,17 +47,8 @@ impl ReporterFactory {
     /// Creates a new TCP-based reporter using the destination from the environment.
     ///
     /// The created reporter is not connected yet; it only stores the destination address.
-    /// It is safe to presume that the existence of the instance does not imply it is
-    /// consuming resources until the `report` method is called.
-    pub fn create() -> Result<tcp::ReporterOnTcp, ReporterCreationError> {
-        let address_str = std::env::var(KEY_DESTINATION)
-            .map_err(|_| ReporterCreationError::MissingEnvironmentVariable(KEY_DESTINATION))?;
-        let address = address_str
-            .parse::<SocketAddr>()
-            .map_err(|_| ReporterCreationError::MissingEnvironmentVariable(KEY_DESTINATION))?;
-
-        let reporter = tcp::ReporterOnTcp::new(address);
-        Ok(reporter)
+    pub fn create(address: SocketAddr) -> impl Reporter {
+        tcp::ReporterOnTcp::new(address)
     }
 
     /// Creates a new reporter and returns it as an atomic pointer.
@@ -74,11 +64,11 @@ impl ReporterFactory {
     ///
     /// Returns a null pointer if reporter creation fails. Caller must check for null
     /// before dereferencing.
-    pub fn create_as_ptr() -> AtomicPtr<tcp::ReporterOnTcp> {
-        match Self::create() {
-            Ok(reporter) => {
+    pub fn create_as_ptr(address_str: &str) -> AtomicPtr<tcp::ReporterOnTcp> {
+        match address_str.parse::<SocketAddr>() {
+            Ok(address) => {
                 // Leak the reporter to get a stable pointer for the lifetime of the program
-                let boxed_reporter = Box::new(reporter);
+                let boxed_reporter = Box::new(tcp::ReporterOnTcp::new(address));
                 let ptr = Box::into_raw(boxed_reporter);
 
                 AtomicPtr::new(ptr)
@@ -90,12 +80,3 @@ impl ReporterFactory {
         }
     }
 }
-
-/// Errors that can occur during reporter initialization.
-#[derive(Error, Debug)]
-pub enum ReporterCreationError {
-    #[error("Environment variable '{0}' is missing")]
-    MissingEnvironmentVariable(&'static str),
-    #[error("Network error: {0}")]
-    Network(#[from] std::io::Error),
-}

intercept-preload/src/implementation.rs

@@ -21,11 +21,12 @@ use std::path::PathBuf;
 use std::ptr;
 use std::sync::atomic::{AtomicPtr, Ordering};
 
+use bear::environment::KEY_DESTINATION;
 use bear::intercept::reporter::{Reporter, ReporterFactory};
 use bear::intercept::tcp::ReporterOnTcp;
 use bear::intercept::{Event, Execution};
 use ctor::ctor;
-use libc::{c_char, c_int, pid_t, posix_spawn_file_actions_t, posix_spawnattr_t};
+use libc::{RTLD_NEXT, c_char, c_int, pid_t, posix_spawn_file_actions_t, posix_spawnattr_t};
 
 /// Constructor function that is called when the library is loaded
 ///
@@ -48,8 +49,6 @@ unsafe fn on_load() {
     log::debug!("Initializing intercept-preload library");
 }
 
-const RTLD_NEXT: *mut libc::c_void = -1isize as *mut libc::c_void;
-
 #[ctor]
 static REAL_EXECVE: AtomicPtr<libc::c_void> = {
     let ptr = unsafe { libc::dlsym(RTLD_NEXT, c"execve".as_ptr() as *const _) };
@@ -112,9 +111,10 @@ static REAL_SYSTEM: AtomicPtr<libc::c_void> = {
 
 #[ctor]
 static REPORTER: AtomicPtr<ReporterOnTcp> = {
-    // This will capture the reporter address for later use, before any environment variable
-    // might be changed.
-    ReporterFactory::create_as_ptr()
+    match std::env::var(KEY_DESTINATION) {
+        Ok(address_str) => ReporterFactory::create_as_ptr(&address_str),
+        Err(_) => AtomicPtr::new(std::ptr::null_mut()),
+    }
 };
 
 /// Rust implementation of execv

intercept-preload/src/lib.rs

@@ -14,4 +14,4 @@ pub use implementation::*;
 
 /// Version information for the library
 #[unsafe(no_mangle)]
-pub static LIBEAR_VERSION: &[u8; 6] = b"4.0.0\0";
+pub static LIBEAR_VERSION: &[u8; 6] = b"4.0.3\0";

intercept-wrapper/src/main.rs

@@ -19,9 +19,8 @@ extern crate core;
 
 use anyhow::{Context, Result};
 use bear::environment::KEY_DESTINATION;
-use bear::intercept::reporter::Reporter;
+use bear::intercept::reporter::{Reporter, ReporterFactory};
 use bear::intercept::supervise::supervise_execution;
-use bear::intercept::tcp;
 use bear::intercept::wrapper::{CONFIG_FILENAME, WrapperConfig, WrapperConfigReader};
 use bear::intercept::{Event, Execution};
 use std::io::Write;
@@ -70,7 +69,7 @@ fn report(real_execution: &Execution) -> Result<()> {
             .parse::<SocketAddr>()
             .with_context(|| format!("Failed to parse interceptor address: {}", address_str))?;
 
-        tcp::ReporterOnTcp::new(address)
+        ReporterFactory::create(address)
     };
     // Report the execution event to collector.
     reporter.report(Event::new(real_execution.clone()))?;