Remove from_env and use env vars by default (#459)

Currently environment variables are not picked up by default and must be opted in via explicit calls to `from_env` in builders. This change updates this behavior by removing the `from_env` methods and instead checking the respective variables in constructors.
open-telemetry · Feb 22, 2021 · 42c37f1 · 42c37f1
1 parent f79586b
commit 42c37f1
Show file tree

Hide file tree

Showing 14 changed files with 176 additions and 222 deletions.
diff --git a/examples/aws-xray/src/client.rs b/examples/aws-xray/src/client.rs
@@ -16,11 +16,11 @@ fn init_tracer() -> (sdktrace::Tracer, stdout::Uninstall) {
     // For the demonstration, use `Sampler::AlwaysOn` sampler to sample all traces. In a production
     // application, use `Sampler::ParentBased` or `Sampler::TraceIdRatioBased` with a desired ratio.
     stdout::new_pipeline()
-        .with_trace_config(sdktrace::Config {
-            default_sampler: Box::new(sdktrace::Sampler::AlwaysOn),
-            id_generator: Box::new(sdktrace::XrayIdGenerator::default()),
-            ..Default::default()
-        })
+        .with_trace_config(
+            sdktrace::config()
+                .with_default_sampler(sdktrace::Sampler::AlwaysOn)
+                .with_id_generator(sdktrace::XrayIdGenerator::default()),
+        )
         .install()
 }
 

diff --git a/examples/aws-xray/src/server.rs b/examples/aws-xray/src/server.rs
@@ -37,11 +37,11 @@ fn init_tracer() -> (sdktrace::Tracer, stdout::Uninstall) {
     // For the demonstration, use `Sampler::AlwaysOn` sampler to sample all traces. In a production
     // application, use `Sampler::ParentBased` or `Sampler::TraceIdRatioBased` with a desired ratio.
     stdout::new_pipeline()
-        .with_trace_config(sdktrace::Config {
-            default_sampler: Box::new(sdktrace::Sampler::AlwaysOn),
-            id_generator: Box::new(sdktrace::XrayIdGenerator::default()),
-            ..Default::default()
-        })
+        .with_trace_config(
+            sdktrace::config()
+                .with_default_sampler(sdktrace::Sampler::AlwaysOn)
+                .with_id_generator(sdktrace::XrayIdGenerator::default()),
+        )
         .install()
 }
 

diff --git a/examples/http/src/client.rs b/examples/http/src/client.rs
@@ -3,7 +3,7 @@ use opentelemetry::global;
 use opentelemetry::sdk::export::trace::stdout;
 use opentelemetry::sdk::{
     propagation::TraceContextPropagator,
-    trace::{Config, Sampler},
+    trace::{self, Sampler},
 };
 use opentelemetry::{
     trace::{TraceContextExt, Tracer},
@@ -17,10 +17,7 @@ fn init_tracer() -> (impl Tracer, stdout::Uninstall) {
     // For the demonstration, use `Sampler::AlwaysOn` sampler to sample all traces. In a production
     // application, use `Sampler::ParentBased` or `Sampler::TraceIdRatioBased` with a desired ratio.
     stdout::new_pipeline()
-        .with_trace_config(Config {
-            default_sampler: Box::new(Sampler::AlwaysOn),
-            ..Default::default()
-        })
+        .with_trace_config(trace::config().with_default_sampler(Sampler::AlwaysOn))
         .install()
 }
 

diff --git a/examples/http/src/server.rs b/examples/http/src/server.rs
@@ -5,7 +5,7 @@ use opentelemetry::{
     sdk::export::trace::stdout,
     sdk::{
         propagation::TraceContextPropagator,
-        trace::{Config, Sampler},
+        trace::{self, Sampler},
     },
     trace::{Span, Tracer},
 };
@@ -29,10 +29,7 @@ fn init_tracer() -> (impl Tracer, stdout::Uninstall) {
     // For the demonstration, use `Sampler::AlwaysOn` sampler to sample all traces. In a production
     // application, use `Sampler::ParentBased` or `Sampler::TraceIdRatioBased` with a desired ratio.
     stdout::new_pipeline()
-        .with_trace_config(Config {
-            default_sampler: Box::new(Sampler::AlwaysOn),
-            ..Default::default()
-        })
+        .with_trace_config(trace::config().with_default_sampler(Sampler::AlwaysOn))
         .install()
 }
 

diff --git a/opentelemetry-jaeger/README.md b/opentelemetry-jaeger/README.md
@@ -77,30 +77,12 @@ opentelemetry-jaeger = { version = "*", features = ["tokio"] }
 
 ### Jaeger Exporter From Environment Variables
 
-The jaeger pipeline builder can be configured dynamically via the [`from_env`]
-method. All variables are optional, a full list of accepted options can be found
-in the [jaeger variables spec].
+The jaeger pipeline builder can be configured dynamically via environment
+variables. All variables are optional, a full list of accepted options can be
+found in the [jaeger variables spec].
 
-[`from_env`]: https://docs.rs/opentelemetry-jaeger/latest/opentelemetry_jaeger/struct.PipelineBuilder.html#method.from_env
 [jaeger variables spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#jaeger-exporter
 
-```rust
-use opentelemetry::global;
-use opentelemetry::trace::Tracer;
-
-fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
-    global::set_text_map_propagator(opentelemetry_jaeger::Propagator::new());
-    // export OTEL_SERVICE_NAME=my-service-name
-    let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline().from_env().install()?;
-
-    tracer.in_span("doing_work", |cx| {
-        // Traced app logic here...
-    });
-
-    Ok(())
-}
-```
-
 ### Jaeger Collector Example
 
 If you want to skip the agent and submit spans directly to a Jaeger collector,
@@ -163,7 +145,6 @@ use opentelemetry::KeyValue;
 fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync + 'static>> {
     global::set_text_map_propagator(opentelemetry_jaeger::Propagator::new());
     let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline()
-        .from_env()
         .with_agent_endpoint("localhost:6831")
         .with_service_name("my_app")
         .with_tags(vec![KeyValue::new("process_key", "process_value")])

diff --git a/opentelemetry-jaeger/src/exporter/mod.rs b/opentelemetry-jaeger/src/exporter/mod.rs
@@ -135,7 +135,7 @@ pub struct PipelineBuilder {
 impl Default for PipelineBuilder {
     /// Return the default Exporter Builder.
     fn default() -> Self {
-        PipelineBuilder {
+        let builder_defaults = PipelineBuilder {
             agent_endpoint: vec![DEFAULT_AGENT_ENDPOINT.parse().unwrap()],
             #[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))]
             collector_endpoint: None,
@@ -151,21 +151,14 @@ impl Default for PipelineBuilder {
                 tags: Vec::new(),
             },
             config: None,
-        }
+        };
+
+        // Override above defaults with env vars if set
+        env::assign_attrs(builder_defaults)
     }
 }
 
 impl PipelineBuilder {
-    /// Assign builder attributes from environment variables.
-    ///
-    /// See the [jaeger variable spec] for full list.
-    ///
-    /// [jaeger variable spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#jaeger-exporter
-    #[allow(clippy::wrong_self_convention)]
-    pub fn from_env(self) -> Self {
-        env::assign_attrs(self)
-    }
-
     /// Assign the agent endpoint.
     pub fn with_agent_endpoint<T: net::ToSocketAddrs>(self, agent_endpoint: T) -> Self {
         PipelineBuilder {

diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs
@@ -53,30 +53,12 @@
 //!
 //! ### Jaeger Exporter From Environment Variables
 //!
-//! The jaeger pipeline builder can be configured dynamically via the
-//! [`from_env`] method. All variables are optinal, a full list of accepted
-//! options can be found in the [jaeger variables spec].
+//! The jaeger pipeline builder can be configured dynamically via environment
+//! variables. All variables are optional, a full list of accepted options can
+//! be found in the [jaeger variables spec].
 //!
-//! [`from_env`]: struct.PipelineBuilder.html#method.from_env
 //! [jaeger variables spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#jaeger-exporter
 //!
-//! ```no_run
-//! use opentelemetry::trace::{Tracer, TraceError};
-//! use opentelemetry::global;
-//!
-//! fn main() -> Result<(), TraceError> {
-//!     global::set_text_map_propagator(opentelemetry_jaeger::Propagator::new());
-//!     // export OTEL_SERVICE_NAME=my-service-name
-//!     let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline().from_env().install()?;
-//!
-//!     tracer.in_span("doing_work", |cx| {
-//!         // Traced app logic here...
-//!     });
-//!
-//!     Ok(())
-//! }
-//! ```
-//!
 //! ### Jaeger Collector Example
 //!
 //! If you want to skip the agent and submit spans directly to a Jaeger collector,
@@ -129,7 +111,6 @@
 //! fn main() -> Result<(), TraceError> {
 //!     global::set_text_map_propagator(opentelemetry_jaeger::Propagator::new());
 //!     let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline()
-//!         .from_env()
 //!         .with_agent_endpoint("localhost:6831")
 //!         .with_service_name("my_app")
 //!         .with_tags(vec![KeyValue::new("process_key", "process_value")])

diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs
@@ -394,7 +394,7 @@ mod tests {
     };
 
     #[test]
-    fn test_pipeline_builder_from_otlp_env() {
+    fn test_pipeline_builder_from_env() {
         std::env::set_var(OTEL_EXPORTER_OTLP_ENDPOINT, "https://otlp_endpoint:4317");
         std::env::set_var(OTEL_EXPORTER_OTLP_TIMEOUT, "bad_timeout");
 
@@ -416,10 +416,8 @@ mod tests {
         std::env::remove_var(OTEL_EXPORTER_OTLP_TIMEOUT);
         assert!(std::env::var(OTEL_EXPORTER_OTLP_ENDPOINT).is_err());
         assert!(std::env::var(OTEL_EXPORTER_OTLP_TIMEOUT).is_err());
-    }
 
-    #[test]
-    fn test_pipeline_builder_from_otlp_traces_env() {
+        // test from traces env var
         std::env::set_var(
             OTEL_EXPORTER_OTLP_TRACES_ENDPOINT,
             "https://otlp_traces_endpoint:4317",

diff --git a/opentelemetry-semantic-conventions/src/resource.rs b/opentelemetry-semantic-conventions/src/resource.rs
@@ -10,16 +10,12 @@
 //! ```rust,no_run
 //! use opentelemetry::sdk;
 //! use opentelemetry_semantic_conventions as semcov;
-//! use std::sync::Arc;
 //!
 //! let _tracer = opentelemetry::sdk::export::trace::stdout::new_pipeline()
-//!     .with_trace_config(sdk::trace::Config {
-//!         resource: Arc::new(sdk::Resource::new(vec![
-//!             semcov::resource::SERVICE_NAME.string("my-service"),
-//!             semcov::resource::SERVICE_NAMESPACE.string("my-namespace"),
-//!         ])),
-//!         ..sdk::trace::Config::default()
-//!     })
+//!     .with_trace_config(sdk::trace::config().with_resource(sdk::Resource::new(vec![
+//!         semcov::resource::SERVICE_NAME.string("my-service"),
+//!         semcov::resource::SERVICE_NAMESPACE.string("my-namespace"),
+//!     ])))
 //!     .install();
 //! ```
 

diff --git a/opentelemetry/src/sdk/resource.rs b/opentelemetry/src/sdk/resource.rs
@@ -15,6 +15,7 @@
 //! [`TracerProvider`]: ../../api/trace/provider/trait.TracerProvider.html
 #[cfg(feature = "metrics")]
 use crate::labels;
+use crate::sdk::EnvResourceDetector;
 use crate::{Key, KeyValue, Value};
 #[cfg(feature = "serialize")]
 use serde::{Deserialize, Serialize};
@@ -25,18 +26,34 @@ use std::time::Duration;
 ///
 /// Items are sorted by their key, and are only overwritten if the value is an empty string.
 #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
-#[derive(Clone, Debug, Default, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct Resource {
     attrs: BTreeMap<Key, Value>,
 }
 
+impl Default for Resource {
+    fn default() -> Self {
+        Self::from_detectors(
+            Duration::from_secs(0),
+            vec![Box::new(EnvResourceDetector::new())],
+        )
+    }
+}
+
 impl Resource {
+    /// Creates an empty resource.
+    pub fn empty() -> Self {
+        Self {
+            attrs: Default::default(),
+        }
+    }
+
     /// Create a new `Resource` from key value pairs.
     ///
     /// Values are de-duplicated by key, and the first key-value pair with a non-empty string value
     /// will be retained
     pub fn new<T: IntoIterator<Item = KeyValue>>(kvs: T) -> Self {
-        let mut resource = Resource::default();
+        let mut resource = Resource::empty();
 
         for kv in kvs.into_iter() {
             resource.insert(kv);
@@ -49,7 +66,7 @@ impl Resource {
     ///
     /// timeout will be applied to each detector.
     pub fn from_detectors(timeout: Duration, detectors: Vec<Box<dyn ResourceDetector>>) -> Self {
-        let mut resource = Resource::default();
+        let mut resource = Resource::empty();
         for detector in detectors {
             let detected_res = detector.detect(timeout);
             for (key, value) in detected_res.into_iter() {
@@ -72,7 +89,7 @@ impl Resource {
             return self.clone();
         }
 
-        let mut resource = Resource::default();
+        let mut resource = Resource::empty();
 
         // attrs from self must be added first so they have priority
         for (k, v) in self.attrs.iter() {

diff --git a/opentelemetry/src/sdk/trace/config.rs b/opentelemetry/src/sdk/trace/config.rs
@@ -3,6 +3,8 @@
 //! Configuration represents the global tracing configuration, overrides
 //! can be set for the default OpenTelemetry limits and Sampler.
 use crate::{sdk, sdk::trace::Sampler, trace::IdGenerator};
+use std::env;
+use std::str::FromStr;
 use std::sync::Arc;
 
 /// Default trace configuration
@@ -71,13 +73,36 @@ impl Config {
 impl Default for Config {
     /// Create default global sdk configuration.
     fn default() -> Self {
-        Config {
+        let mut config = Config {
             default_sampler: Box::new(Sampler::ParentBased(Box::new(Sampler::AlwaysOn))),
             id_generator: Box::new(sdk::trace::IdGenerator::default()),
             max_events_per_span: 128,
-            max_attributes_per_span: 32,
-            max_links_per_span: 32,
+            max_attributes_per_span: 128,
+            max_links_per_span: 128,
             resource: Arc::new(sdk::Resource::default()),
+        };
+
+        if let Some(max_attributes_per_span) = env::var("OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT")
+            .ok()
+            .and_then(|count_limit| u32::from_str(&count_limit).ok())
+        {
+            config.max_attributes_per_span = max_attributes_per_span;
+        }
+
+        if let Some(max_events_per_span) = env::var("OTEL_SPAN_EVENT_COUNT_LIMIT")
+            .ok()
+            .and_then(|max_events| u32::from_str(&max_events).ok())
+        {
+            config.max_events_per_span = max_events_per_span;
         }
+
+        if let Some(max_links_per_span) = env::var("OTEL_SPAN_LINK_COUNT_LIMIT")
+            .ok()
+            .and_then(|max_links| u32::from_str(&max_links).ok())
+        {
+            config.max_links_per_span = max_links_per_span;
+        }
+
+        config
     }
 }
diff --git a/opentelemetry/src/sdk/trace/id_generator/aws.rs b/opentelemetry/src/sdk/trace/id_generator/aws.rs
@@ -24,14 +24,11 @@ use std::time::{Duration, UNIX_EPOCH};
 ///
 /// ```
 /// use opentelemetry::trace::NoopSpanExporter;
-/// use opentelemetry::sdk::trace::{Config, TracerProvider, XrayIdGenerator};
+/// use opentelemetry::sdk::trace::{self, TracerProvider, XrayIdGenerator};
 ///
 /// let _provider: TracerProvider = TracerProvider::builder()
 ///     .with_simple_exporter(NoopSpanExporter::new())
-///     .with_config(Config {
-///          id_generator: Box::new(XrayIdGenerator::default()),
-///          ..Default::default()
-///     })
+///     .with_config(trace::config().with_id_generator(XrayIdGenerator::default()))
 ///     .build();
 /// ```
 ///

diff --git a/opentelemetry/src/sdk/trace/provider.rs b/opentelemetry/src/sdk/trace/provider.rs
@@ -116,11 +116,11 @@ impl Builder {
         // drop. We cannot assume we are in a multi-threaded tokio runtime here, so use
         // `spawn_blocking` to avoid blocking the main thread.
         let spawn = |fut| tokio::task::spawn_blocking(|| futures::executor::block_on(fut));
-        let batch = sdk::trace::BatchSpanProcessor::from_env(
+        let batch = sdk::trace::BatchSpanProcessor::builder(
             exporter,
             spawn,
-            crate::util::tokio_interval_stream,
             tokio::time::sleep,
+            crate::util::tokio_interval_stream,
         );
         self.with_batch_exporter(batch.build())
     }
@@ -129,11 +129,11 @@ impl Builder {
     #[cfg(all(feature = "async-std", not(feature = "tokio")))]
     #[cfg_attr(docsrs, doc(cfg(feature = "async-std")))]
     pub fn with_exporter<T: SpanExporter + 'static>(self, exporter: T) -> Self {
-        let batch = sdk::trace::BatchSpanProcessor::from_env(
+        let batch = sdk::trace::BatchSpanProcessor::builder(
             exporter,
             async_std::task::spawn,
-            async_std::stream::interval,
             async_std::task::sleep,
+            async_std::stream::interval,
         );
         self.with_batch_exporter(batch.build())
     }