diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 0026252710..bda3ee592c 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -2,7 +2,9 @@ ## vNext -- *Breaking* Make `force_flush()` in `PushMetricExporter` synchronous +- Calls to `MeterProviderBuilder::with_resource`, `TracerProviderBuilder::with_resource`, + `LoggerProviderBuilder::with_resource` are now additive ([#2677](https://github.com/open-telemetry/opentelemetry-rust/pull/2677)). +- *Breaking*: Make `force_flush()` in `PushMetricExporter` synchronous ## 0.28.0 diff --git a/opentelemetry-sdk/src/logs/logger_provider.rs b/opentelemetry-sdk/src/logs/logger_provider.rs index 23b35e8f72..7ec8c8dfd3 100644 --- a/opentelemetry-sdk/src/logs/logger_provider.rs +++ b/opentelemetry-sdk/src/logs/logger_provider.rs @@ -235,11 +235,16 @@ impl LoggerProviderBuilder { } /// The `Resource` to be associated with this Provider. + /// + /// *Note*: Calls to this method are additive, each call merges the provided + /// resource with the previous one. pub fn with_resource(self, resource: Resource) -> Self { - LoggerProviderBuilder { - resource: Some(resource), - ..self - } + let resource = match self.resource { + Some(existing) => Some(existing.merge(&resource)), + None => Some(resource), + }; + + LoggerProviderBuilder { resource, ..self } } /// Create a new provider from this configuration. @@ -712,6 +717,35 @@ mod tests { assert_eq!(scoped_logger.instrumentation_scope().name(), ""); } + #[test] + fn with_resource_multiple_calls_ensure_additive() { + let builder = SdkLoggerProvider::builder() + .with_resource(Resource::new(vec![KeyValue::new("key1", "value1")])) + .with_resource(Resource::new(vec![KeyValue::new("key2", "value2")])) + .with_resource( + Resource::builder_empty() + .with_schema_url(vec![], "http://example.com") + .build(), + ) + .with_resource(Resource::new(vec![KeyValue::new("key3", "value3")])); + + let resource = builder.resource.unwrap(); + + assert_eq!( + resource.get(&Key::from_static_str("key1")), + Some(Value::from("value1")) + ); + assert_eq!( + resource.get(&Key::from_static_str("key2")), + Some(Value::from("value2")) + ); + assert_eq!( + resource.get(&Key::from_static_str("key3")), + Some(Value::from("value3")) + ); + assert_eq!(resource.schema_url(), Some("http://example.com")); + } + #[derive(Debug)] pub(crate) struct LazyLogProcessor { shutdown_called: Arc>, diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 1e0d02a84d..5474316235 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -233,9 +233,16 @@ impl MeterProviderBuilder { /// /// By default, if this option is not used, the default [Resource] will be used. /// + /// *Note*: Calls to this method are additive, each call merges the provided + /// resource with the previous one. + /// /// [Meter]: opentelemetry::metrics::Meter pub fn with_resource(mut self, resource: Resource) -> Self { - self.resource = Some(resource); + self.resource = match self.resource { + Some(existing) => Some(existing.merge(&resource)), + None => Some(resource), + }; + self } @@ -322,6 +329,7 @@ impl fmt::Debug for MeterProviderBuilder { #[cfg(all(test, feature = "testing"))] mod tests { use crate::error::OTelSdkError; + use crate::metrics::SdkMeterProvider; use crate::resource::{ SERVICE_NAME, TELEMETRY_SDK_LANGUAGE, TELEMETRY_SDK_NAME, TELEMETRY_SDK_VERSION, }; @@ -553,4 +561,33 @@ mod tests { assert_eq!(provider.inner.meters.lock().unwrap().len(), 5); } + + #[test] + fn with_resource_multiple_calls_ensure_additive() { + let builder = SdkMeterProvider::builder() + .with_resource(Resource::new(vec![KeyValue::new("key1", "value1")])) + .with_resource(Resource::new(vec![KeyValue::new("key2", "value2")])) + .with_resource( + Resource::builder_empty() + .with_schema_url(vec![], "http://example.com") + .build(), + ) + .with_resource(Resource::new(vec![KeyValue::new("key3", "value3")])); + + let resource = builder.resource.unwrap(); + + assert_eq!( + resource.get(&Key::from_static_str("key1")), + Some(Value::from("value1")) + ); + assert_eq!( + resource.get(&Key::from_static_str("key2")), + Some(Value::from("value2")) + ); + assert_eq!( + resource.get(&Key::from_static_str("key3")), + Some(Value::from("value3")) + ); + assert_eq!(resource.schema_url(), Some("http://example.com")); + } } diff --git a/opentelemetry-sdk/src/resource/mod.rs b/opentelemetry-sdk/src/resource/mod.rs index aa2a101c01..bf6f43dc1b 100644 --- a/opentelemetry-sdk/src/resource/mod.rs +++ b/opentelemetry-sdk/src/resource/mod.rs @@ -169,11 +169,11 @@ impl Resource { /// 5. If both resources do not have a schema url, the schema url will be empty. /// /// [Schema url]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.9.0/specification/schemas/overview.md#schema-url - fn merge>(&self, other: T) -> Self { - if self.is_empty() { + pub(crate) fn merge>(&self, other: T) -> Self { + if self.is_empty() && self.schema_url().is_none() { return other.clone(); } - if other.is_empty() { + if other.is_empty() && other.schema_url().is_none() { return self.clone(); } let mut combined_attrs = self.inner.attrs.clone(); @@ -419,18 +419,29 @@ mod tests { } #[rstest] - #[case(vec![], vec![KeyValue::new("key", "b")], "http://schema/a", None)] - #[case(vec![KeyValue::new("key", "a")], vec![KeyValue::new("key", "b")], "http://schema/a", Some("http://schema/a"))] - fn merge_resource_with_missing_attribtes( + #[case(vec![], vec![KeyValue::new("key", "b")], Some("http://schema/a"), None, Some("http://schema/a"))] + #[case(vec![KeyValue::new("key", "a")], vec![KeyValue::new("key", "b")], Some("http://schema/a"), None, Some("http://schema/a"))] + #[case(vec![KeyValue::new("key", "a")], vec![KeyValue::new("key", "b")], Some("http://schema/a"), None, Some("http://schema/a"))] + #[case(vec![KeyValue::new("key", "a")], vec![KeyValue::new("key", "b")], Some("http://schema/a"), Some("http://schema/b"), None)] + #[case(vec![KeyValue::new("key", "a")], vec![KeyValue::new("key", "b")], None, Some("http://schema/b"), Some("http://schema/b"))] + fn merge_resource_with_missing_attributes( #[case] key_values_a: Vec, #[case] key_values_b: Vec, - #[case] schema_url: &'static str, + #[case] schema_url_a: Option<&'static str>, + #[case] schema_url_b: Option<&'static str>, #[case] expected_schema_url: Option<&'static str>, ) { - let resource = Resource::from_schema_url(key_values_a, schema_url); - let other_resource = Resource::builder_empty() - .with_attributes(key_values_b) - .build(); + let resource = match schema_url_a { + Some(schema) => Resource::from_schema_url(key_values_a, schema), + None => Resource::new(key_values_a), + }; + + let other_resource = match schema_url_b { + Some(schema) => Resource::builder_empty() + .with_schema_url(key_values_b, schema) + .build(), + None => Resource::new(key_values_b), + }; assert_eq!( resource.merge(&other_resource).schema_url(), diff --git a/opentelemetry-sdk/src/trace/provider.rs b/opentelemetry-sdk/src/trace/provider.rs index 84c67637ad..6951b6dbd5 100644 --- a/opentelemetry-sdk/src/trace/provider.rs +++ b/opentelemetry-sdk/src/trace/provider.rs @@ -293,6 +293,7 @@ impl opentelemetry::trace::TracerProvider for SdkTracerProvider { pub struct TracerProviderBuilder { processors: Vec>, config: crate::trace::Config, + resource: Option, } impl TracerProviderBuilder { @@ -410,18 +411,28 @@ impl TracerProviderBuilder { /// /// By default, if this option is not used, the default [Resource] will be used. /// + /// *Note*: Calls to this method are additive, each call merges the provided + /// resource with the previous one. + /// /// [Tracer]: opentelemetry::trace::Tracer pub fn with_resource(self, resource: Resource) -> Self { - TracerProviderBuilder { - config: self.config.with_resource(resource), - ..self - } + let resource = match self.resource { + Some(existing) => Some(existing.merge(&resource)), + None => Some(resource), + }; + + TracerProviderBuilder { resource, ..self } } /// Create a new provider from this configuration. pub fn build(self) -> SdkTracerProvider { let mut config = self.config; + // Now, we can update the config with the resource. + if let Some(resource) = self.resource { + config = config.with_resource(resource); + }; + // Standard config will contain an owned [`Resource`] (either sdk default or use supplied) // we can optimize the common case with a static ref to avoid cloning the underlying // resource data for each span. @@ -462,8 +473,8 @@ mod tests { SERVICE_NAME, TELEMETRY_SDK_LANGUAGE, TELEMETRY_SDK_NAME, TELEMETRY_SDK_VERSION, }; use crate::trace::provider::TracerProviderInner; - use crate::trace::SpanData; use crate::trace::{Config, Span, SpanProcessor}; + use crate::trace::{SdkTracerProvider, SpanData}; use crate::Resource; use opentelemetry::trace::{Tracer, TracerProvider}; use opentelemetry::{Context, Key, KeyValue, Value}; @@ -730,6 +741,39 @@ mod tests { assert!(test_tracer_1.provider().is_shutdown()); } + #[test] + fn with_resource_multiple_calls_ensure_additive() { + let resource = SdkTracerProvider::builder() + .with_resource(Resource::new(vec![KeyValue::new("key1", "value1")])) + .with_resource(Resource::new(vec![KeyValue::new("key2", "value2")])) + .with_resource( + Resource::builder_empty() + .with_schema_url(vec![], "http://example.com") + .build(), + ) + .with_resource(Resource::new(vec![KeyValue::new("key3", "value3")])) + .build() + .inner + .config + .resource + .clone() + .into_owned(); + + assert_eq!( + resource.get(&Key::from_static_str("key1")), + Some(Value::from("value1")) + ); + assert_eq!( + resource.get(&Key::from_static_str("key2")), + Some(Value::from("value2")) + ); + assert_eq!( + resource.get(&Key::from_static_str("key3")), + Some(Value::from("value3")) + ); + assert_eq!(resource.schema_url(), Some("http://example.com")); + } + #[derive(Debug)] struct CountingShutdownProcessor { shutdown_count: Arc,