From 8d258731449cf34417ae38a000410614753731c7 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbone Date: Thu, 29 Jan 2026 21:56:19 -0500 Subject: [PATCH 1/3] refactor: make reqwest available in common --- Cargo.lock | 5 + libdd-common-ffi/src/slice.rs | 4 +- libdd-common-ffi/src/slice_mut.rs | 4 +- libdd-common/Cargo.toml | 16 ++- libdd-common/src/cc_utils.rs | 3 +- .../src/dump_server.rs | 4 +- libdd-common/src/lib.rs | 85 ++++++++++- .../src/test_utils.rs | 133 ++++++++++++++++-- libdd-common/tests/reqwest_builder_test.rs | 79 +++++++++++ libdd-profiling/Cargo.toml | 3 +- libdd-profiling/src/exporter/mod.rs | 3 - .../src/exporter/profile_exporter.rs | 50 +------ libdd-profiling/tests/exporter_e2e.rs | 12 +- libdd-profiling/tests/file_exporter_test.rs | 65 ++------- 14 files changed, 332 insertions(+), 134 deletions(-) rename libdd-profiling/src/exporter/file_exporter.rs => libdd-common/src/dump_server.rs (98%) rename libdd-profiling/src/exporter/utils.rs => libdd-common/src/test_utils.rs (56%) create mode 100644 libdd-common/tests/reqwest_builder_test.rs diff --git a/Cargo.lock b/Cargo.lock index 6a7065dfc0..df2f70e0aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2986,15 +2986,20 @@ dependencies = [ "http", "http-body", "http-body-util", + "httparse", "hyper 1.6.0", "hyper-rustls", "hyper-util", "indexmap 2.12.1", "libc", "maplit", + "mime 0.3.17", + "multipart", "nix", "pin-project", + "rand 0.8.5", "regex", + "reqwest", "rustls", "rustls-native-certs", "serde", diff --git a/libdd-common-ffi/src/slice.rs b/libdd-common-ffi/src/slice.rs index a86b3398bc..a703188e1b 100644 --- a/libdd-common-ffi/src/slice.rs +++ b/libdd-common-ffi/src/slice.rs @@ -404,7 +404,7 @@ mod tests { len: 0, _marker: PhantomData, }; - assert_eq!(null_len0.as_slice(), &[]); + assert_eq!(null_len0.as_slice(), &[] as &[u8]); } #[should_panic] @@ -447,7 +447,7 @@ mod tests { }; let result = null_zero_len.try_as_slice(); - assert_eq!(result.unwrap(), &[]); + assert_eq!(result.unwrap(), &[] as &[u8]); } #[test] diff --git a/libdd-common-ffi/src/slice_mut.rs b/libdd-common-ffi/src/slice_mut.rs index a45520a5ef..60bea136f2 100644 --- a/libdd-common-ffi/src/slice_mut.rs +++ b/libdd-common-ffi/src/slice_mut.rs @@ -238,7 +238,7 @@ mod tests { len: 0, _marker: PhantomData, }; - assert_eq!(null_len0.as_mut_slice(), &[]); + assert_eq!(null_len0.as_mut_slice(), &[] as &[u8]); } #[should_panic] @@ -281,7 +281,7 @@ mod tests { }; let result = null_zero_len.try_as_slice(); - assert_eq!(result.unwrap(), &[]); + assert_eq!(result.unwrap(), &[] as &[u8]); } #[test] diff --git a/libdd-common/Cargo.toml b/libdd-common/Cargo.toml index dfdd0f3aef..3dd8231b2a 100644 --- a/libdd-common/Cargo.toml +++ b/libdd-common/Cargo.toml @@ -21,6 +21,7 @@ futures = "0.3" futures-core = { version = "0.3.0", default-features = false } futures-util = { version = "0.3.0", default-features = false } hex = "0.4" +httparse = { version = "1.9", optional = true } hyper = { workspace = true } hyper-util = { workspace = true } http = "1.0" @@ -28,11 +29,15 @@ http-body = "1.0" http-body-util = "0.1" tower-service = "0.3" cc = "1.1.31" +mime = { version = "0.3.16", optional = true } +multipart = { version = "0.18", optional = true } pin-project = "1" +rand = { version = "0.8", optional = true } regex = "1.5" +reqwest = { version = "0.13", features = ["rustls"], default-features = false, optional = true } rustls-native-certs = { version = "0.8.1", optional = true } thiserror = "1.0" -tokio = { version = "1.23", features = ["rt", "macros"] } +tokio = { version = "1.23", features = ["rt", "macros", "net", "io-util", "fs"] } tokio-rustls = { version = "0.26", default-features = false, optional = true } serde = { version = "1.0", features = ["derive"] } static_assertions = "1.1.0" @@ -62,9 +67,14 @@ hyper-rustls = { version = "0.27", default-features = false, features = [ ], optional = true } [dev-dependencies] +httparse = "1.9" indexmap = "2.11" maplit = "1.0" +mime = "0.3.16" +multipart = "0.18" +rand = "0.8" tempfile = "3.8" +tokio = { version = "1.23", features = ["rt", "macros", "time"] } [features] default = ["https"] @@ -75,6 +85,10 @@ use_webpki_roots = ["hyper-rustls/webpki-roots"] cgroup_testing = [] # FIPS mode uses the FIPS-compliant cryptographic provider (Unix only) fips = ["https", "hyper-rustls/fips"] +# Enable reqwest client builder support with file dump debugging +reqwest = ["dep:reqwest", "test-utils"] +# Enable test utilities for use in other crates +test-utils = ["dep:httparse", "dep:rand", "dep:mime", "dep:multipart"] [lints.rust] # We run coverage checks in our github actions. These checks are run with diff --git a/libdd-common/src/cc_utils.rs b/libdd-common/src/cc_utils.rs index 637870872d..71e9e91779 100644 --- a/libdd-common/src/cc_utils.rs +++ b/libdd-common/src/cc_utils.rs @@ -1,6 +1,7 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 +use anyhow::Context; use std::{ env, ffi::{self, OsString}, @@ -122,7 +123,7 @@ impl ImprovedBuild { fn get_out_dir(&self) -> anyhow::Result { env::var_os("OUT_DIR") .map(PathBuf::from) - .ok_or_else(|| anyhow::Error::msg("can't get output directory info")) + .context("can't get output directory info") } // cc::Build shadow diff --git a/libdd-profiling/src/exporter/file_exporter.rs b/libdd-common/src/dump_server.rs similarity index 98% rename from libdd-profiling/src/exporter/file_exporter.rs rename to libdd-common/src/dump_server.rs index 1a11655c3e..66dd352e3c 100644 --- a/libdd-profiling/src/exporter/file_exporter.rs +++ b/libdd-common/src/dump_server.rs @@ -32,7 +32,7 @@ const HTTP_200_RESPONSE: &[u8] = b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n" /// # Returns /// The path to the Unix socket (on Unix) or named pipe (on Windows) that the server is listening on #[cfg(unix)] -pub(crate) fn spawn_dump_server(output_path: PathBuf) -> anyhow::Result { +pub fn spawn_dump_server(output_path: PathBuf) -> anyhow::Result { use tokio::net::UnixListener; // Create a temporary socket path with randomness to avoid collisions @@ -86,7 +86,7 @@ pub(crate) fn spawn_dump_server(output_path: PathBuf) -> anyhow::Result /// # Returns /// The path to the Windows named pipe that the server is listening on #[cfg(windows)] -pub(crate) fn spawn_dump_server(output_path: PathBuf) -> anyhow::Result { +pub fn spawn_dump_server(output_path: PathBuf) -> anyhow::Result { use tokio::net::windows::named_pipe::ServerOptions; // Create a unique named pipe name with randomness to avoid collisions diff --git a/libdd-common/src/lib.rs b/libdd-common/src/lib.rs index 6ec3934a9d..815a2b15e0 100644 --- a/libdd-common/src/lib.rs +++ b/libdd-common/src/lib.rs @@ -6,6 +6,7 @@ #![cfg_attr(not(test), deny(clippy::todo))] #![cfg_attr(not(test), deny(clippy::unimplemented))] +use anyhow::Context; use hyper::{ header::HeaderValue, http::uri::{self}, @@ -18,6 +19,8 @@ use std::{borrow::Cow, ops::Deref, path::PathBuf, str::FromStr}; pub mod azure_app_services; pub mod cc_utils; pub mod connector; +#[cfg(feature = "reqwest")] +pub mod dump_server; pub mod entity_id; #[macro_use] pub mod cstr; @@ -26,6 +29,8 @@ pub mod error; pub mod hyper_migration; pub mod rate_limiter; pub mod tag; +#[cfg(any(test, feature = "test-utils"))] +pub mod test_utils; pub mod timeout; pub mod unix_utils; pub mod worker; @@ -220,11 +225,7 @@ fn encode_uri_path_in_authority(scheme: &str, path: &str) -> anyhow::Result anyhow::Result { - let path = hex::decode( - uri.authority() - .ok_or_else(|| anyhow::anyhow!("missing uri authority"))? - .as_str(), - )?; + let path = hex::decode(uri.authority().context("missing uri authority")?.as_str())?; #[cfg(unix)] { use std::os::unix::ffi::OsStringExt; @@ -320,4 +321,78 @@ impl Endpoint { }; self } + + /// Creates a reqwest ClientBuilder configured for this endpoint. + /// + /// This method handles various endpoint schemes: + /// - `http`/`https`: Standard HTTP(S) endpoints + /// - `unix`: Unix domain sockets (Unix only) + /// - `windows`: Windows named pipes (Windows only) + /// - `file`: File dump endpoints for debugging (spawns a local server to capture requests) + /// + /// # Returns + /// A tuple of (ClientBuilder, request_url) where: + /// - ClientBuilder is configured with the appropriate transport and timeout + /// - request_url is the URL string to use for HTTP requests + /// + /// # Errors + /// Returns an error if: + /// - The endpoint scheme is unsupported + /// - Path decoding fails + /// - The dump server fails to start (for file:// scheme) + #[cfg(feature = "reqwest")] + pub fn to_reqwest_client_builder(&self) -> anyhow::Result<(reqwest::ClientBuilder, String)> { + use anyhow::Context; + + let mut builder = + reqwest::Client::builder().timeout(std::time::Duration::from_millis(self.timeout_ms)); + + let request_url = match self.url.scheme_str() { + // HTTP/HTTPS endpoints + Some("http") | Some("https") => self.url.to_string(), + + // File dump endpoint (debugging) - uses platform-specific local transport + Some("file") => { + let output_path = decode_uri_path_in_authority(&self.url) + .context("Failed to decode file path from URI")?; + let socket_or_pipe_path = dump_server::spawn_dump_server(output_path)?; + + // Configure the client to use the local socket/pipe + #[cfg(unix)] + { + builder = builder.unix_socket(socket_or_pipe_path); + } + #[cfg(windows)] + { + builder = builder + .windows_named_pipe(socket_or_pipe_path.to_string_lossy().to_string()); + } + + "http://localhost/".to_string() + } + + // Unix domain sockets + #[cfg(unix)] + Some("unix") => { + use connector::uds::socket_path_from_uri; + let socket_path = socket_path_from_uri(&self.url)?; + builder = builder.unix_socket(socket_path); + format!("http://localhost{}", self.url.path()) + } + + // Windows named pipes + #[cfg(windows)] + Some("windows") => { + use connector::named_pipe::named_pipe_path_from_uri; + let pipe_path = named_pipe_path_from_uri(&self.url)?; + builder = builder.windows_named_pipe(pipe_path.to_string_lossy().to_string()); + format!("http://localhost{}", self.url.path()) + } + + // Unsupported schemes + scheme => anyhow::bail!("Unsupported endpoint scheme: {:?}", scheme), + }; + + Ok((builder, request_url)) + } } diff --git a/libdd-profiling/src/exporter/utils.rs b/libdd-common/src/test_utils.rs similarity index 56% rename from libdd-profiling/src/exporter/utils.rs rename to libdd-common/src/test_utils.rs index 82e0969232..e1b6badebf 100644 --- a/libdd-profiling/src/exporter/utils.rs +++ b/libdd-common/src/test_utils.rs @@ -1,12 +1,84 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -//! Utility functions for parsing and inspecting HTTP requests and multipart form data. +//! Common test utilities for libdatadog crates. //! -//! These utilities are primarily useful for testing and debugging profiling exports. +//! This module provides shared helper functions and types for testing, +//! including file cleanup, HTTP parsing, and temp file management. -use anyhow::Context as _; +use anyhow::Context; use std::collections::HashMap; +use std::path::PathBuf; + +/// RAII guard to ensure test files are cleaned up even if the test panics +/// +/// # Example +/// ```no_run +/// use libdd_common::test_utils::TempFileGuard; +/// use std::path::PathBuf; +/// +/// let file = TempFileGuard::new(PathBuf::from("/tmp/test.txt")); +/// // Use file... +/// // File is automatically deleted when guard goes out of scope +/// ``` +pub struct TempFileGuard(PathBuf); + +impl TempFileGuard { + /// Create a new temp file guard for the given path + pub fn new(path: PathBuf) -> Self { + Self(path) + } +} + +impl Drop for TempFileGuard { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.0); + } +} + +impl std::ops::Deref for TempFileGuard { + type Target = PathBuf; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl AsRef for TempFileGuard { + fn as_ref(&self) -> &std::path::Path { + self.0.as_ref() + } +} + +/// Create a unique temporary file path with the given prefix +/// +/// The path will be in the system temp directory with a unique name based on +/// process ID and a random number to avoid collisions. +/// +/// # Arguments +/// * `prefix` - Prefix for the temp file name +/// * `extension` - File extension (without the dot) +/// +/// # Returns +/// A `TempFileGuard` that will automatically clean up the file when dropped +/// +/// # Example +/// ```no_run +/// use libdd_common::test_utils::create_temp_file_path; +/// +/// let file = create_temp_file_path("test", "txt"); +/// // file path is something like /tmp/test_12345_abc123.txt +/// ``` +pub fn create_temp_file_path(prefix: &str, extension: &str) -> TempFileGuard { + let file_path = std::env::temp_dir().join(format!( + "{}_{}_{:x}.{}", + prefix, + std::process::id(), + rand::random::(), + extension + )); + TempFileGuard::new(file_path) +} /// Represents a parsed HTTP request #[derive(Debug)] @@ -40,11 +112,12 @@ pub struct MultipartPart { /// /// # Example /// ```no_run -/// use libdd_profiling::exporter::utils::parse_http_request; +/// use libdd_common::test_utils::parse_http_request; /// -/// let request_bytes = b"POST /v1/input HTTP/1.1\r\nHost: example.com\r\n\r\n"; +/// let request_bytes = b"POST /v1/input HTTP/1.1\r\nHost: example.com\r\n\r\nbody"; /// let request = parse_http_request(request_bytes).unwrap(); /// assert_eq!(request.method, "POST"); +/// assert_eq!(request.path, "/v1/input"); /// ``` pub fn parse_http_request(data: &[u8]) -> anyhow::Result { let mut header_buf = [httparse::EMPTY_HEADER; 64]; @@ -62,7 +135,7 @@ pub fn parse_http_request(data: &[u8]) -> anyhow::Result { let mut headers = HashMap::new(); for header in req.headers { let key = header.name.to_lowercase(); - let value = std::str::from_utf8(header.value)?.to_string(); + let value = String::from_utf8_lossy(header.value).into_owned(); headers.insert(key, value); } @@ -83,10 +156,9 @@ pub fn parse_http_request(data: &[u8]) -> anyhow::Result { }) } -/// Parse multipart form data from Content-Type header and body (internal helper) +/// Parse multipart form data from Content-Type header and body /// /// Extracts the boundary from the Content-Type header and parses the multipart body. -/// This is called automatically by `parse_http_request` when appropriate. fn parse_multipart(content_type: &str, body: &[u8]) -> anyhow::Result> { use multipart::server::Multipart; use std::io::Cursor; @@ -94,7 +166,7 @@ fn parse_multipart(content_type: &str, body: &[u8]) -> anyhow::Result anyhow::Result { + Ok(client + .post(url) + .header("Content-Type", "text/plain") + .header("X-Test-Header", "test-value") + .body(body.to_string()) + .send() + .await?) + } + + #[tokio::test] + async fn test_file_dump_captures_http_request() { + let file_path = create_temp_file_path("libdd_common_test", "http"); + + // Create endpoint with file:// scheme + let endpoint = Endpoint::from_slice(&format!("file://{}", file_path.display())); + + // Build reqwest client + let (builder, url) = endpoint + .to_reqwest_client_builder() + .expect("should build client"); + let client = builder.build().expect("should create client"); + + // Send a simple request + let test_body = "Hello from test!"; + let response = send_request(client, &url, test_body) + .await + .expect("request should succeed"); + + assert_eq!(response.status(), 200); + + // Read the captured request + // No sleep needed - the server only sends 200 after file.sync_all() completes + let captured = std::fs::read(&*file_path).expect("should read dump file"); + + // Parse and validate + let request = parse_http_request(&captured).expect("should parse captured request"); + + assert_eq!(request.method, "POST"); + assert_eq!(request.path, "/"); + + // Find our custom headers + assert_eq!( + request.headers.get("content-type").map(|s| s.as_str()), + Some("text/plain") + ); + assert_eq!( + request.headers.get("x-test-header").map(|s| s.as_str()), + Some("test-value") + ); + + // Validate body + assert_eq!(request.body, test_body.as_bytes()); + } + + #[tokio::test] + async fn test_unsupported_scheme_returns_error() { + let endpoint = Endpoint::from_slice("ftp://example.com/file"); + + let result = endpoint.to_reqwest_client_builder(); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert!(err.to_string().contains("Unsupported endpoint scheme")); + } +} diff --git a/libdd-profiling/Cargo.toml b/libdd-profiling/Cargo.toml index 61acf19d89..87ec4ae9d6 100644 --- a/libdd-profiling/Cargo.toml +++ b/libdd-profiling/Cargo.toml @@ -42,7 +42,7 @@ http-body-util = "0.1" httparse = "1.9" indexmap = "2.11" libdd-alloc = { version = "1.0.0", path = "../libdd-alloc" } -libdd-common = { version = "1.1.0", path = "../libdd-common", default-features = false } +libdd-common = { version = "1.1.0", path = "../libdd-common", default-features = false, features = ["reqwest", "test-utils"] } libdd-profiling-protobuf = { version = "1.0.0", path = "../libdd-profiling-protobuf", features = ["prost_impls"] } mime = "0.3.16" multipart = { version = "0.18", optional = true } @@ -65,6 +65,7 @@ zstd = { version = "0.13", default-features = false } [dev-dependencies] bolero = "0.13" criterion = "0.5.1" +libdd-common = { path = "../libdd-common", features = ["test-utils"] } libdd-profiling = { path = ".", features = ["test-utils"] } proptest = "1" tempfile = "3" diff --git a/libdd-profiling/src/exporter/mod.rs b/libdd-profiling/src/exporter/mod.rs index 2f073f038c..e2dce20b3b 100644 --- a/libdd-profiling/src/exporter/mod.rs +++ b/libdd-profiling/src/exporter/mod.rs @@ -4,10 +4,7 @@ pub mod config; mod errors; pub mod exporter_manager; -mod file_exporter; mod profile_exporter; -#[cfg(any(test, feature = "test-utils"))] -pub mod utils; pub use errors::SendError; pub use exporter_manager::ExporterManager; diff --git a/libdd-profiling/src/exporter/profile_exporter.rs b/libdd-profiling/src/exporter/profile_exporter.rs index efc9a54907..7799f4c395 100644 --- a/libdd-profiling/src/exporter/profile_exporter.rs +++ b/libdd-profiling/src/exporter/profile_exporter.rs @@ -25,7 +25,6 @@ //! which can be useful for debugging or replaying requests. use super::errors::SendError; -use super::file_exporter::spawn_dump_server; use anyhow::Context; use libdd_common::tag::Tag; use libdd_common::{azure_app_services, tag, Endpoint}; @@ -98,54 +97,7 @@ impl ProfileExporter { mut tags: Vec, endpoint: Endpoint, ) -> anyhow::Result { - let mut builder = reqwest::Client::builder() - .timeout(std::time::Duration::from_millis(endpoint.timeout_ms)); - - let request_url = match endpoint.url.scheme_str() { - // HTTP/HTTPS endpoints - Some("http") | Some("https") => endpoint.url.to_string(), - - // File dump endpoint (debugging) - uses platform-specific local transport - Some("file") => { - let output_path = libdd_common::decode_uri_path_in_authority(&endpoint.url) - .context("Failed to decode file path from URI")?; - let socket_or_pipe_path = spawn_dump_server(output_path)?; - - // Configure the client to use the local socket/pipe - #[cfg(unix)] - { - builder = builder.unix_socket(socket_or_pipe_path); - } - #[cfg(windows)] - { - builder = builder - .windows_named_pipe(socket_or_pipe_path.to_string_lossy().to_string()); - } - - "http://localhost/".to_string() - } - - // Unix domain sockets - #[cfg(unix)] - Some("unix") => { - use libdd_common::connector::uds::socket_path_from_uri; - let socket_path = socket_path_from_uri(&endpoint.url)?; - builder = builder.unix_socket(socket_path); - format!("http://localhost{}", endpoint.url.path()) - } - - // Windows named pipes - #[cfg(windows)] - Some("windows") => { - use libdd_common::connector::named_pipe::named_pipe_path_from_uri; - let pipe_path = named_pipe_path_from_uri(&endpoint.url)?; - builder = builder.windows_named_pipe(pipe_path.to_string_lossy().to_string()); - format!("http://localhost{}", endpoint.url.path()) - } - - // Unsupported schemes - scheme => anyhow::bail!("Unsupported endpoint scheme: {:?}", scheme), - }; + let (builder, request_url) = endpoint.to_reqwest_client_builder()?; // Pre-build all static headers let mut headers = reqwest::header::HeaderMap::new(); diff --git a/libdd-profiling/tests/exporter_e2e.rs b/libdd-profiling/tests/exporter_e2e.rs index 3bca79c48e..4125c947a0 100644 --- a/libdd-profiling/tests/exporter_e2e.rs +++ b/libdd-profiling/tests/exporter_e2e.rs @@ -5,8 +5,8 @@ //! //! These tests validate the full export flow across different endpoint types. +use libdd_common::test_utils::parse_http_request; use libdd_profiling::exporter::config; -use libdd_profiling::exporter::utils::parse_http_request; use libdd_profiling::exporter::{File, MimeType, ProfileExporter}; use libdd_profiling::internal::EncodedProfile; use std::collections::HashMap; @@ -126,9 +126,8 @@ async fn spawn_server(transport: Transport) -> anyhow::Result { } }); - // Give the server a moment to start listening - tokio::time::sleep(tokio::time::Duration::from_millis(10)).await; - + // No sleep needed - create() synchronously creates the pipe and makes it ready, + // just like bind() for TCP/UDS Ok(ServerInfo { port: None, #[cfg(unix)] @@ -276,7 +275,7 @@ async fn export_full_profile( // Get the request from the appropriate source match source { RequestSource::File(path) => { - tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; + // No sleep needed - send_blocking() waits for file to be synced let request_bytes = std::fs::read(&path)?; let req = parse_http_request(&request_bytes)?; Ok(ReceivedRequest { @@ -287,7 +286,8 @@ async fn export_full_profile( }) } RequestSource::Captured(requests) => { - tokio::time::sleep(tokio::time::Duration::from_millis(50)).await; + // No sleep needed - send().await waits until HTTP response is received, + // which means the server has already captured the request let reqs = requests.lock().unwrap(); if reqs.is_empty() { anyhow::bail!("No request captured"); diff --git a/libdd-profiling/tests/file_exporter_test.rs b/libdd-profiling/tests/file_exporter_test.rs index e6912ecda5..224349c817 100644 --- a/libdd-profiling/tests/file_exporter_test.rs +++ b/libdd-profiling/tests/file_exporter_test.rs @@ -1,33 +1,9 @@ // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ // SPDX-License-Identifier: Apache-2.0 -use libdd_profiling::exporter::utils::parse_http_request; +use libdd_common::test_utils::{create_temp_file_path, parse_http_request, TempFileGuard}; use libdd_profiling::exporter::ProfileExporter; use libdd_profiling::internal::EncodedProfile; -use std::path::PathBuf; - -/// RAII guard to ensure test files are cleaned up even if the test panics -struct TempFileGuard(PathBuf); - -impl Drop for TempFileGuard { - fn drop(&mut self) { - let _ = std::fs::remove_file(&self.0); - } -} - -impl std::ops::Deref for TempFileGuard { - type Target = PathBuf; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl AsRef for TempFileGuard { - fn as_ref(&self) -> &std::path::Path { - self.0.as_ref() - } -} /// Create a file-based exporter and return the temp file path with auto-cleanup fn create_file_exporter( @@ -40,12 +16,7 @@ fn create_file_exporter( use libdd_profiling::exporter::config; // Create a unique temp file path - let file_path = std::env::temp_dir().join(format!( - "libdd_test_{}_{}_{:x}.http", - std::process::id(), - chrono::Utc::now().timestamp_nanos_opt().unwrap_or(0), - rand::random::() - )); + let file_path = create_temp_file_path("libdd_profiling_test", "http"); let mut endpoint = config::file(file_path.to_string_lossy().as_ref())?; if let Some(key) = api_key { @@ -60,7 +31,7 @@ fn create_file_exporter( endpoint, )?; - Ok((exporter, TempFileGuard(file_path))) + Ok((exporter, file_path)) } #[cfg(test)] @@ -94,10 +65,8 @@ mod tests { .send_blocking(profile, &[], &[], None, None, None, None) .expect("send to succeed"); - // Read the dump file (wait a moment for it to be written) - // The file is synced before the 200 response, but we still need a small delay - // to ensure the background thread's runtime has fully completed the async operation - std::thread::sleep(std::time::Duration::from_millis(200)); + // Read the dump file + // send_blocking() blocks until the request completes and file is synced let request_bytes = std::fs::read(&file_path).expect("read dump file"); // Parse HTTP request @@ -206,10 +175,8 @@ mod tests { ) .expect("send to succeed"); - // Read the dump file (wait a moment for it to be written) - // The file is synced before the 200 response, but we still need a small delay - // to ensure the background thread's runtime has fully completed the async operation - std::thread::sleep(std::time::Duration::from_millis(200)); + // Read the dump file + // send_blocking() blocks until the request completes and file is synced let request_bytes = std::fs::read(&file_path).expect("read dump file"); // Parse and validate @@ -257,10 +224,8 @@ mod tests { ) .expect("send to succeed"); - // Read the dump file (wait a moment for it to be written) - // The file is synced before the 200 response, but we still need a small delay - // to ensure the background thread's runtime has fully completed the async operation - std::thread::sleep(std::time::Duration::from_millis(200)); + // Read the dump file + // send_blocking() blocks until the request completes and file is synced let request_bytes = std::fs::read(&file_path).expect("read dump file"); // Parse and validate @@ -315,10 +280,8 @@ mod tests { .send_blocking(profile, &[], &[], None, Some(info.clone()), None, None) .expect("send to succeed"); - // Read the dump file (wait a moment for it to be written) - // The file is synced before the 200 response, but we still need a small delay - // to ensure the background thread's runtime has fully completed the async operation - std::thread::sleep(std::time::Duration::from_millis(200)); + // Read the dump file + // send_blocking() blocks until the request completes and file is synced let request_bytes = std::fs::read(&file_path).expect("read dump file"); // Parse and validate @@ -357,10 +320,8 @@ mod tests { .send_blocking(profile, &[], &[], None, None, None, None) .expect("send to succeed"); - // Read the dump file (wait a moment for it to be written) - // The file is synced before the 200 response, but we still need a small delay - // to ensure the background thread's runtime has fully completed the async operation - std::thread::sleep(std::time::Duration::from_millis(200)); + // Read the dump file + // send_blocking() blocks until the request completes and file is synced let request_bytes = std::fs::read(&file_path).expect("read dump file"); // Parse HTTP request From fb7155454151255859d200a76873f285f204a4f7 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbone Date: Tue, 3 Feb 2026 13:34:23 -0500 Subject: [PATCH 2/3] fix miri --- libdd-common/tests/reqwest_builder_test.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libdd-common/tests/reqwest_builder_test.rs b/libdd-common/tests/reqwest_builder_test.rs index 6df3910866..14abbdc5d1 100644 --- a/libdd-common/tests/reqwest_builder_test.rs +++ b/libdd-common/tests/reqwest_builder_test.rs @@ -22,6 +22,7 @@ mod tests { } #[tokio::test] + #[cfg_attr(miri, ignore)] async fn test_file_dump_captures_http_request() { let file_path = create_temp_file_path("libdd_common_test", "http"); @@ -67,6 +68,7 @@ mod tests { } #[tokio::test] + #[cfg_attr(miri, ignore)] async fn test_unsupported_scheme_returns_error() { let endpoint = Endpoint::from_slice("ftp://example.com/file"); From aef72ec4062709e0b8c9d9e18bff6dc10d60ca77 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbone Date: Tue, 3 Feb 2026 15:37:03 -0500 Subject: [PATCH 3/3] PR comment --- libdd-common/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libdd-common/src/lib.rs b/libdd-common/src/lib.rs index 815a2b15e0..bf2a67c790 100644 --- a/libdd-common/src/lib.rs +++ b/libdd-common/src/lib.rs @@ -7,10 +7,7 @@ #![cfg_attr(not(test), deny(clippy::unimplemented))] use anyhow::Context; -use hyper::{ - header::HeaderValue, - http::uri::{self}, -}; +use hyper::{header::HeaderValue, http::uri}; use serde::de::Error; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use std::sync::{Mutex, MutexGuard};