move trace_util test helpers behind a feature flag (#461)

Put trace_test_utils behind a test-utils feature flag

To reduce the possibility of test helper functions being used in
non-test code. Also, move create_send_data and poll_for_mock_hit
functions into trace_test_utils. And update third party license file for test-utils feature in trace-utils crate

sidecar/Cargo.toml

@@ -72,11 +72,6 @@ features = [
 ]
 version = "0.51.0"
 
-[dev-dependencies]
-libc = { version = "0.2" }
-tempfile = { version = "3.3" }
-httpmock = "0.7.0"
-
 [target.'cfg(not(target_arch = "x86"))'.dependencies]
 simd-json = "0.13.8"
 
@@ -90,3 +85,10 @@ kernel32-sys = "0.2.2"
 
 [target.'cfg(windows_seh_wrapper)'.dependencies]
 microseh = "0.1.1"
+
+[dev-dependencies]
+libc = { version = "0.2" }
+tempfile = { version = "3.3" }
+httpmock = "0.7.0"
+datadog-trace-utils = { path = "../trace-utils", features = ["test-utils"] }
+

sidecar/src/service/tracing/trace_flusher.rs

@@ -311,56 +311,10 @@ impl TraceFlusher {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use datadog_trace_protobuf::pb;
-    use datadog_trace_utils::trace_utils::TracerHeaderTags;
-    use httpmock::{Mock, MockServer};
+    use datadog_trace_utils::test_utils::{create_send_data, poll_for_mock_hit};
+    use httpmock::MockServer;
     use std::sync::Arc;
 
-    // This function will poll the mock server for "hits" until the expected number of hits is
-    // observed. Then it will delete the mock. In its current form it may not correctly report if
-    // more than the asserted number of hits occurred. More attempts at lower sleep intervals is
-    // preferred to reduce flakiness and test runtime.
-    async fn poll_for_mock_hit(
-        mock: &mut Mock<'_>,
-        poll_attempts: i32,
-        sleep_interval_ms: u64,
-        expected_hits: usize,
-    ) -> bool {
-        let mut mock_hit = mock.hits_async().await == expected_hits;
-
-        let mut mock_observations_remaining = poll_attempts;
-
-        while !mock_hit {
-            tokio::time::sleep(Duration::from_millis(sleep_interval_ms)).await;
-            mock_hit = mock.hits_async().await == expected_hits;
-            mock_observations_remaining -= 1;
-            if mock_observations_remaining == 0 || mock_hit {
-                break;
-            }
-        }
-
-        mock_hit
-    }
-
-    fn create_send_data(size: usize, target_endpoint: &Endpoint) -> SendData {
-        let tracer_header_tags = TracerHeaderTags::default();
-
-        let tracer_payload = pb::TracerPayload {
-            container_id: "container_id_1".to_owned(),
-            language_name: "php".to_owned(),
-            language_version: "4.0".to_owned(),
-            tracer_version: "1.1".to_owned(),
-            runtime_id: "runtime_1".to_owned(),
-            chunks: vec![],
-            tags: Default::default(),
-            env: "test".to_owned(),
-            hostname: "test_host".to_owned(),
-            app_version: "2.0".to_owned(),
-        };
-
-        SendData::new(size, tracer_payload, tracer_header_tags, target_endpoint)
-    }
-
     #[cfg_attr(miri, ignore)]
     #[tokio::test]
     // Test scenario: Enqueue two traces with a size less than the minimum force flush size, and
@@ -397,12 +351,12 @@ mod tests {
         trace_flusher.enqueue(send_data_1);
         trace_flusher.enqueue(send_data_2);
 
-        assert!(poll_for_mock_hit(&mut mock, 10, 150, 0).await);
+        assert!(poll_for_mock_hit(&mut mock, 10, 150, 0, false).await);
 
         // enqueue a trace that exceeds the min force flush size
         trace_flusher.enqueue(send_data_3);
 
-        assert!(poll_for_mock_hit(&mut mock, 25, 100, 1).await);
+        assert!(poll_for_mock_hit(&mut mock, 25, 100, 1, true).await);
     }
 
     #[cfg_attr(miri, ignore)]
@@ -438,7 +392,7 @@ mod tests {
             trace_flusher.interval_ms.load(Ordering::Relaxed) + 1,
         ))
         .await;
-        assert!(poll_for_mock_hit(&mut mock, 25, 100, 1).await);
+        assert!(poll_for_mock_hit(&mut mock, 25, 100, 1, true).await);
     }
 
     #[cfg_attr(miri, ignore)]
@@ -470,6 +424,6 @@ mod tests {
 
         trace_flusher.enqueue(send_data_1);
 
-        assert!(poll_for_mock_hit(&mut mock, 5, 250, 0).await);
+        assert!(poll_for_mock_hit(&mut mock, 5, 250, 0, true).await);
     }
 }

trace-mini-agent/Cargo.toml

@@ -23,3 +23,4 @@ rmp-serde = "1.1.1"
 serial_test = "2.0.0"
 duplicate = "0.4.1"
 tempfile = "3.3.0"
+datadog-trace-utils = { path = "../trace-utils", features=["test-utils"] }

trace-mini-agent/src/trace_processor.rs

@@ -124,7 +124,7 @@ mod tests {
     };
     use datadog_trace_protobuf::pb;
     use datadog_trace_utils::{
-        trace_test_utils::{create_test_json_span, create_test_span},
+        test_utils::{create_test_json_span, create_test_span},
         trace_utils,
     };
     use ddcommon::Endpoint;

trace-obfuscation/src/obfuscate.rs

@@ -50,15 +50,15 @@ pub fn obfuscate_span(span: &mut pb::Span, config: &ObfuscationConfig) {
 
 #[cfg(test)]
 mod tests {
-    use datadog_trace_utils::trace_test_utils;
+    use datadog_trace_utils::test_utils;
 
     use crate::{obfuscation_config, replacer};
 
     use super::obfuscate_span;
 
     #[test]
     fn test_obfuscates_span_url_strings() {
-        let mut span = trace_test_utils::create_test_span(111, 222, 0, 1, true);
+        let mut span = test_utils::create_test_span(111, 222, 0, 1, true);
         span.r#type = "http".to_string();
         span.meta.insert(
             "http.url".to_string(),
@@ -81,7 +81,7 @@ mod tests {
 
     #[test]
     fn test_replace_span_tags() {
-        let mut span = trace_test_utils::create_test_span(111, 222, 0, 1, true);
+        let mut span = test_utils::create_test_span(111, 222, 0, 1, true);
         span.meta
             .insert("custom.tag".to_string(), "/foo/bar/foo".to_string());
 
@@ -105,7 +105,7 @@ mod tests {
 
     #[test]
     fn obfuscate_all_redis_args() {
-        let mut span = trace_test_utils::create_test_span(111, 222, 0, 1, true);
+        let mut span = test_utils::create_test_span(111, 222, 0, 1, true);
         span.r#type = "redis".to_string();
         span.meta.insert(
             "redis.raw_command".to_string(),
@@ -125,7 +125,7 @@ mod tests {
 
     #[test]
     fn obfuscate_redis_raw_query() {
-        let mut span = trace_test_utils::create_test_span(111, 222, 0, 1, true);
+        let mut span = test_utils::create_test_span(111, 222, 0, 1, true);
         span.r#type = "redis".to_string();
         span.meta.insert(
             "redis.raw_command".to_string(),

trace-utils/Cargo.toml

@@ -23,8 +23,12 @@ datadog-trace-normalization = { path = "../trace-normalization" }
 tokio = { version = "1", features = ["macros", "rt-multi-thread"] }
 rand = "0.8.5"
 bytes = "1.6.0"
+# This should only be used for testing. It isn't under dev-dependencies because test-utils can't be under #[cfg(test)].
+httpmock = {  version = "0.7.0", optional = true}
 
 [dev-dependencies]
 tokio = { version = "1", features = ["macros", "rt-multi-thread"] }
-httpmock = "0.7.0"
 serde_json = "1.0"
+
+[features]
+test-utils = ["httpmock"]

trace-utils/src/lib.rs

@@ -4,6 +4,7 @@
 pub mod config_utils;
 pub mod send_data;
 pub mod stats_utils;
-pub mod trace_test_utils;
+#[cfg(any(test, feature = "test-utils"))]
+pub mod test_utils;
 pub mod trace_utils;
 pub mod tracer_header_tags;

trace-utils/src/send_data.rs

@@ -534,14 +534,12 @@ impl SendData {
 // For RetryStrategy tests the observed delay should be approximate.
 mod tests {
     use super::*;
-    use crate::trace_utils::construct_trace_chunk;
-    use crate::trace_utils::construct_tracer_payload;
-    use crate::trace_utils::RootSpanTags;
+    use crate::test_utils::{create_send_data, poll_for_mock_hit};
+    use crate::trace_utils::{construct_trace_chunk, construct_tracer_payload, RootSpanTags};
     use crate::tracer_header_tags::TracerHeaderTags;
     use datadog_trace_protobuf::pb;
     use ddcommon::Endpoint;
     use httpmock::prelude::*;
-    use httpmock::Mock;
     use httpmock::MockServer;
     use std::collections::HashMap;
     use tokio::time::Instant;
@@ -1029,57 +1027,6 @@ mod tests {
         );
     }
 
-    // TODO: APMSP-1153 - This function also exists in
-    // sidecar::service::tracing::trace_flusher::tests. It should be moved to a common
-    // trace_test_utils module when it is properly gated to just test dependency.
-    async fn poll_for_mock_hit(
-        mock: &mut Mock<'_>,
-        poll_attempts: i32,
-        sleep_interval_ms: u64,
-        expected_hits: usize,
-        delete_after_hit: bool,
-    ) -> bool {
-        let mut mock_hit = mock.hits_async().await == expected_hits;
-
-        let mut mock_observations_remaining = poll_attempts;
-
-        while !mock_hit {
-            sleep(Duration::from_millis(sleep_interval_ms)).await;
-            mock_hit = mock.hits_async().await == expected_hits;
-            mock_observations_remaining -= 1;
-            if mock_observations_remaining == 0 || mock_hit {
-                if delete_after_hit {
-                    mock.delete();
-                }
-                break;
-            }
-        }
-
-        mock_hit
-    }
-
-    // TODO: APMSP-1153 - This function also exists in
-    // sidecar::service::tracing::trace_flusher::tests. It should be moved to a common
-    // trace_test_utils module when it is properly gated to just test dependency.
-    fn create_send_data(size: usize, target_endpoint: &Endpoint) -> SendData {
-        let tracer_header_tags = TracerHeaderTags::default();
-
-        let tracer_payload = pb::TracerPayload {
-            container_id: "container_id_1".to_owned(),
-            language_name: "php".to_owned(),
-            language_version: "4.0".to_owned(),
-            tracer_version: "1.1".to_owned(),
-            runtime_id: "runtime_1".to_owned(),
-            chunks: vec![],
-            tags: Default::default(),
-            env: "test".to_owned(),
-            hostname: "test_host".to_owned(),
-            app_version: "2.0".to_owned(),
-        };
-
-        SendData::new(size, tracer_payload, tracer_header_tags, target_endpoint)
-    }
-
     #[cfg_attr(miri, ignore)]
     #[tokio::test]
     async fn test_zero_retries_on_error() {