fix(sdk, zipkin, otlp): use temp_env for testing (#1403)

Fixes #1383

Author: TommyCpp

opentelemetry-otlp/Cargo.toml

@@ -52,6 +52,7 @@ opentelemetry_sdk = { features = ["trace", "rt-tokio", "testing"], path = "../op
 time = { version = "0.3", features = ["macros"] }
 tokio = { version = "1.0", features = ["macros", "rt-multi-thread"] }
 futures-util = { version = "0.3", default-features = false }
+temp-env = "0.3.6"
 
 [features]
 # telemetry pillars and functions

opentelemetry-otlp/src/exporter/mod.rs

@@ -243,23 +243,20 @@ fn parse_header_key_value_string(key_value_string: &str) -> Option<(&str, &str)>
 #[cfg(test)]
 #[cfg(any(feature = "grpc-tonic", feature = "http-proto"))]
 mod tests {
-    // Make sure env tests are not running concurrently
-    static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
 
     pub(crate) fn run_env_test<T, F>(env_vars: T, f: F)
     where
         F: FnOnce(),
         T: Into<Vec<(&'static str, &'static str)>>,
     {
-        let _env_lock = ENV_LOCK.lock().expect("env test lock poisoned");
-        let env_vars = env_vars.into();
-        for (k, v) in env_vars.iter() {
-            std::env::set_var(k, v);
-        }
-        f();
-        for (k, _) in env_vars {
-            std::env::remove_var(k);
-        }
+        temp_env::with_vars(
+            env_vars
+                .into()
+                .iter()
+                .map(|&(k, v)| (k, Some(v)))
+                .collect::<Vec<(&'static str, Option<&'static str>)>>(),
+            f,
+        )
     }
 
     #[test]

opentelemetry-sdk/Cargo.toml

@@ -38,6 +38,8 @@ rustdoc-args = ["--cfg", "docsrs"]
 [dev-dependencies]
 indexmap = "2.0"
 criterion = { version = "0.5", features = ["html_reports"] }
+temp-env = "0.3.6"
+
 [target.'cfg(not(target_os = "windows"))'.dev-dependencies]
 pprof = { version = "0.13", features = ["flamegraph", "criterion"] }
 

opentelemetry-sdk/src/metrics/meter_provider.rs

@@ -247,51 +247,54 @@ mod tests {
             .build();
         assert_service_name(custom_meter_provider, Some("test_service"));
 
-        // If `OTEL_RESOURCE_ATTRIBUTES` is set, read them automatically
-        let reader3 = TestMetricReader {};
-        env::set_var("OTEL_RESOURCE_ATTRIBUTES", "key1=value1, k2, k3=value2");
-        let env_resource_provider = super::SdkMeterProvider::builder()
-            .with_reader(reader3)
-            .build();
-        assert_eq!(
-            env_resource_provider.pipes.0[0].resource,
-            Resource::new(vec![
-                KeyValue::new("telemetry.sdk.name", "opentelemetry"),
-                KeyValue::new("telemetry.sdk.version", env!("CARGO_PKG_VERSION")),
-                KeyValue::new("telemetry.sdk.language", "rust"),
-                KeyValue::new("key1", "value1"),
-                KeyValue::new("k3", "value2"),
-                KeyValue::new("service.name", "unknown_service"),
-            ])
+        temp_env::with_var(
+            "OTEL_RESOURCE_ATTRIBUTES",
+            Some("key1=value1, k2, k3=value2"),
+            || {
+                // If `OTEL_RESOURCE_ATTRIBUTES` is set, read them automatically
+                let reader3 = TestMetricReader {};
+                let env_resource_provider = super::SdkMeterProvider::builder()
+                    .with_reader(reader3)
+                    .build();
+                assert_eq!(
+                    env_resource_provider.pipes.0[0].resource,
+                    Resource::new(vec![
+                        KeyValue::new("telemetry.sdk.name", "opentelemetry"),
+                        KeyValue::new("telemetry.sdk.version", env!("CARGO_PKG_VERSION")),
+                        KeyValue::new("telemetry.sdk.language", "rust"),
+                        KeyValue::new("key1", "value1"),
+                        KeyValue::new("k3", "value2"),
+                        KeyValue::new("service.name", "unknown_service"),
+                    ])
+                );
+            },
         );
 
         // When `OTEL_RESOURCE_ATTRIBUTES` is set and also user provided config
-        env::set_var(
+        temp_env::with_var(
             "OTEL_RESOURCE_ATTRIBUTES",
-            "my-custom-key=env-val,k2=value2",
-        );
-        let reader4 = TestMetricReader {};
-        let user_provided_resource_config_provider = super::SdkMeterProvider::builder()
-            .with_reader(reader4)
-            .with_resource(
-                Resource::default().merge(&mut Resource::new(vec![KeyValue::new(
-                    "my-custom-key",
-                    "my-custom-value",
-                )])),
-            )
-            .build();
-        assert_eq!(
-            user_provided_resource_config_provider.pipes.0[0].resource,
-            Resource::new(vec![
-                KeyValue::new("telemetry.sdk.name", "opentelemetry"),
-                KeyValue::new("telemetry.sdk.version", env!("CARGO_PKG_VERSION")),
-                KeyValue::new("telemetry.sdk.language", "rust"),
-                KeyValue::new("my-custom-key", "my-custom-value"),
-                KeyValue::new("k2", "value2"),
-                KeyValue::new("service.name", "unknown_service"),
-            ])
+            Some("my-custom-key=env-val,k2=value2"),
+            || {
+                let reader4 = TestMetricReader {};
+                let user_provided_resource_config_provider = super::SdkMeterProvider::builder()
+                    .with_reader(reader4)
+                    .with_resource(Resource::default().merge(&mut Resource::new(vec![
+                        KeyValue::new("my-custom-key", "my-custom-value"),
+                    ])))
+                    .build();
+                assert_eq!(
+                    user_provided_resource_config_provider.pipes.0[0].resource,
+                    Resource::new(vec![
+                        KeyValue::new("telemetry.sdk.name", "opentelemetry"),
+                        KeyValue::new("telemetry.sdk.version", env!("CARGO_PKG_VERSION")),
+                        KeyValue::new("telemetry.sdk.language", "rust"),
+                        KeyValue::new("my-custom-key", "my-custom-value"),
+                        KeyValue::new("k2", "value2"),
+                        KeyValue::new("service.name", "unknown_service"),
+                    ])
+                );
+            },
         );
-        env::remove_var("OTEL_RESOURCE_ATTRIBUTES");
 
         // If user provided a resource, it takes priority during collision.
         let reader5 = TestMetricReader {};

opentelemetry-sdk/src/resource/env.rs

@@ -99,68 +99,80 @@ mod tests {
     use crate::resource::{EnvResourceDetector, Resource, ResourceDetector};
     use opentelemetry::{Key, KeyValue, Value};
     use std::time::Duration;
-    use std::{env, time};
 
     #[test]
     fn test_read_from_env() {
-        env::set_var(OTEL_RESOURCE_ATTRIBUTES, "key=value, k = v , a= x, a=z");
-        env::set_var("irrelevant".to_uppercase(), "20200810");
-
-        let detector = EnvResourceDetector::new();
-        let resource = detector.detect(time::Duration::from_secs(5));
-        assert_eq!(
-            resource,
-            Resource::new(vec![
-                KeyValue::new("key", "value"),
-                KeyValue::new("k", "v"),
-                KeyValue::new("a", "x"),
-                KeyValue::new("a", "z"),
-            ])
+        temp_env::with_vars(
+            [
+                (
+                    "OTEL_RESOURCE_ATTRIBUTES",
+                    Some("key=value, k = v , a= x, a=z"),
+                ),
+                ("IRRELEVANT", Some("20200810")),
+            ],
+            || {
+                let detector = EnvResourceDetector::new();
+                let resource = detector.detect(Duration::from_secs(5));
+                assert_eq!(
+                    resource,
+                    Resource::new(vec![
+                        KeyValue::new("key", "value"),
+                        KeyValue::new("k", "v"),
+                        KeyValue::new("a", "x"),
+                        KeyValue::new("a", "z"),
+                    ])
+                );
+            },
         );
 
-        // Test this case in same test to avoid race condition when running tests in parallel.
-        env::set_var(OTEL_RESOURCE_ATTRIBUTES, "");
-
         let detector = EnvResourceDetector::new();
-        let resource = detector.detect(time::Duration::from_secs(5));
+        let resource = detector.detect(Duration::from_secs(5));
         assert!(resource.is_empty());
     }
 
     #[test]
     fn test_sdk_provided_resource_detector() {
         const SERVICE_NAME: &str = "service.name";
         // Ensure no env var set
-        env::remove_var(OTEL_RESOURCE_ATTRIBUTES);
         let no_env = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
         assert_eq!(
             no_env.get(Key::from_static_str(SERVICE_NAME)),
             Some(Value::from("unknown_service")),
         );
 
-        env::set_var(OTEL_SERVICE_NAME, "test service");
-        let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
-        assert_eq!(
-            with_service.get(Key::from_static_str(SERVICE_NAME)),
-            Some(Value::from("test service")),
-        );
-        env::set_var(OTEL_SERVICE_NAME, ""); // clear the env var
-
-        // Fall back to OTEL_RESOURCE_ATTRIBUTES
-        env::set_var(OTEL_RESOURCE_ATTRIBUTES, "service.name=test service1");
-        let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
-        assert_eq!(
-            with_service.get(Key::from_static_str(SERVICE_NAME)),
-            Some(Value::from("test service1"))
+        temp_env::with_var(OTEL_SERVICE_NAME, Some("test service"), || {
+            let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
+            assert_eq!(
+                with_service.get(Key::from_static_str(SERVICE_NAME)),
+                Some(Value::from("test service")),
+            )
+        });
+
+        temp_env::with_var(
+            OTEL_RESOURCE_ATTRIBUTES,
+            Some("service.name=test service1"),
+            || {
+                let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
+                assert_eq!(
+                    with_service.get(Key::from_static_str(SERVICE_NAME)),
+                    Some(Value::from("test service1")),
+                )
+            },
         );
 
         // OTEL_SERVICE_NAME takes priority
-        env::set_var(OTEL_SERVICE_NAME, "test service");
-        let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
-        assert_eq!(
-            with_service.get(Key::from_static_str(SERVICE_NAME)),
-            Some(Value::from("test service"))
+        temp_env::with_vars(
+            [
+                (OTEL_SERVICE_NAME, Some("test service")),
+                (OTEL_RESOURCE_ATTRIBUTES, Some("service.name=test service3")),
+            ],
+            || {
+                let with_service = SdkProvidedResourceDetector.detect(Duration::from_secs(1));
+                assert_eq!(
+                    with_service.get(Key::from_static_str(SERVICE_NAME)),
+                    Some(Value::from("test service"))
+                );
+            },
         );
-        env::set_var(OTEL_RESOURCE_ATTRIBUTES, "");
-        env::set_var(OTEL_SERVICE_NAME, ""); // clear the env var
     }
 }

opentelemetry-sdk/src/resource/mod.rs

@@ -255,7 +255,7 @@ mod tests {
     use super::*;
     use crate::resource::EnvResourceDetector;
     use std::collections::HashMap;
-    use std::{env, time};
+    use std::time;
 
     #[test]
     fn new_resource() {
@@ -339,20 +339,30 @@ mod tests {
 
     #[test]
     fn detect_resource() {
-        env::set_var("OTEL_RESOURCE_ATTRIBUTES", "key=value, k = v , a= x, a=z");
-        env::set_var("irrelevant".to_uppercase(), "20200810");
-
-        let detector = EnvResourceDetector::new();
-        let resource =
-            Resource::from_detectors(time::Duration::from_secs(5), vec![Box::new(detector)]);
-        assert_eq!(
-            resource,
-            Resource::new(vec![
-                KeyValue::new("key", "value"),
-                KeyValue::new("k", "v"),
-                KeyValue::new("a", "x"),
-                KeyValue::new("a", "z"),
-            ])
+        temp_env::with_vars(
+            [
+                (
+                    "OTEL_RESOURCE_ATTRIBUTES",
+                    Some("key=value, k = v , a= x, a=z"),
+                ),
+                ("IRRELEVANT", Some("20200810")),
+            ],
+            || {
+                let detector = EnvResourceDetector::new();
+                let resource = Resource::from_detectors(
+                    time::Duration::from_secs(5),
+                    vec![Box::new(detector)],
+                );
+                assert_eq!(
+                    resource,
+                    Resource::new(vec![
+                        KeyValue::new("key", "value"),
+                        KeyValue::new("k", "v"),
+                        KeyValue::new("a", "x"),
+                        KeyValue::new("a", "z"),
+                    ])
+                )
+            },
         )
     }
 }