From d42d1c1ddd48e7f811c0ffa5a8d770dc0f7757f1 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 20 May 2025 23:02:38 -0700 Subject: [PATCH 1/5] feat: Use builder pattern for Streams --- examples/metrics-advanced/src/main.rs | 13 +- opentelemetry-sdk/CHANGELOG.md | 9 +- opentelemetry-sdk/src/metrics/instrument.rs | 202 +++++++++++++----- .../src/metrics/meter_provider.rs | 80 ++++++- opentelemetry-sdk/src/metrics/mod.rs | 95 +++++--- opentelemetry-sdk/src/metrics/view.rs | 15 +- 6 files changed, 320 insertions(+), 94 deletions(-) diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index 19c0cd0e89..00b8044698 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -9,9 +9,11 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { let my_view_rename_and_unit = |i: &Instrument| { if i.name == "my_histogram" { Some( - Stream::new() - .name("my_histogram_renamed") - .unit("milliseconds"), + Stream::builder() + .with_name("my_histogram_renamed") + .with_unit("milliseconds") + .build() + .unwrap(), ) } else { None @@ -21,7 +23,10 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { // for example 2 let my_view_change_cardinality = |i: &Instrument| { if i.name == "my_second_histogram" { - Some(Stream::new().cardinality_limit(2)) + // Note: If Stream is invalid, build() will return an error. + // By calling `.ok()`, any such error is ignored and treated as if the view does not exist. + // Consider handling the error explicitly if this is not the desired behavior. + Stream::builder().with_cardinality_limit(0).build().ok() } else { None } diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index a062bf23bf..d943b42709 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -42,14 +42,17 @@ also modified to suppress telemetry before invoking exporters. behind feature flag "experimental_metrics_custom_reader". [#2928](https://github.com/open-telemetry/opentelemetry-rust/pull/2928) -- TODO: Placeholder for View related changelog. Polish this after all - - The `Stream` struct now has its public fields hidden. +- TODO: Placeholder for View related changelog. Polish this after all changs done - Core view functionality is now available by default—users can change the name, unit, description, and cardinality limit of a metric via views without enabling the `spec_unstable_metrics_views` feature flag. Advanced view features, such as custom aggregation or attribute filtering, still require the `spec_unstable_metrics_views` feature. - - TODO: Add Stream::builder() pattern change, validation when done. +- Introduced a builder pattern for `Stream` creation to use with "Views". + - Added `StreamBuilder` struct with methods to configure stream properties + - Added `Stream::builder()` method that returns a new `StreamBuilder` + - `StreamBuilder::build()` returns `Result>` enabling + proper validation - *Breaking* `Aggregation` enum moved behind feature flag "spec_unstable_metrics_views". This was only required when using Views. diff --git a/opentelemetry-sdk/src/metrics/instrument.rs b/opentelemetry-sdk/src/metrics/instrument.rs index 2fffbeca42..ffb7b320ee 100644 --- a/opentelemetry-sdk/src/metrics/instrument.rs +++ b/opentelemetry-sdk/src/metrics/instrument.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, collections::HashSet, sync::Arc}; +use std::{borrow::Cow, collections::HashSet, error::Error, sync::Arc}; use opentelemetry::{ metrics::{AsyncInstrument, SyncInstrument}, @@ -73,10 +73,13 @@ impl InstrumentKind { /// Instruments can be used as criteria for views. /// /// ``` -/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, Stream}; +/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, StreamBuilder}; /// /// let criteria = Instrument::new().name("counter_*"); -/// let mask = Stream::new().aggregation(Aggregation::Sum); +/// let mask = Stream::builder() +/// .with_aggregation(Aggregation::Sum) +/// .build() +/// .unwrap(); /// /// let view = new_view(criteria, mask); /// # drop(view); @@ -169,71 +172,60 @@ impl Instrument { } } -/// Describes the stream of data an instrument produces. +/// A builder for creating Stream objects. /// /// # Example /// -/// Streams can be used as masks in views. -/// /// ``` -/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, Stream}; +/// use opentelemetry_sdk::metrics::{Aggregation, StreamBuilder}; +/// use opentelemetry::Key; /// -/// let criteria = Instrument::new().name("counter_*"); -/// let mask = Stream::new().aggregation(Aggregation::Sum); -/// -/// let view = new_view(criteria, mask); -/// # drop(view); +/// let stream = StreamBuilder::new() +/// .with_name("my_stream") +/// .with_aggregation(Aggregation::Sum) +/// .with_cardinality_limit(100) +/// .build() +/// .unwrap(); /// ``` #[derive(Default, Debug)] #[non_exhaustive] -#[allow(unreachable_pub)] -pub struct Stream { - /// The human-readable identifier of the stream. - pub(crate) name: Option>, - /// Describes the purpose of the data. - pub(crate) description: Option>, - /// the unit of measurement recorded. - pub(crate) unit: Option>, - /// Aggregation the stream uses for an instrument. - pub(crate) aggregation: Option, - /// An allow-list of attribute keys that will be preserved for the stream. - /// - /// Any attribute recorded for the stream with a key not in this set will be - /// dropped. If the set is empty, all attributes will be dropped, if `None` all - /// attributes will be kept. - pub(crate) allowed_attribute_keys: Option>>, - - /// Cardinality limit for the stream. - pub(crate) cardinality_limit: Option, +pub struct StreamBuilder { + name: Option>, + description: Option>, + unit: Option>, + aggregation: Option, + allowed_attribute_keys: Option>>, + cardinality_limit: Option, } -impl Stream { - /// Create a new stream with empty values. - pub fn new() -> Self { - Stream::default() +impl StreamBuilder { + /// Create a new stream builder with default values. + pub(crate) fn new() -> Self { + StreamBuilder::default() } - /// Set the stream name. - pub fn name(mut self, name: impl Into>) -> Self { + /// Set the stream name. If this is not set, name provide while creating the instrument will be used. + pub fn with_name(mut self, name: impl Into>) -> Self { self.name = Some(name.into()); self } - /// Set the stream description. - pub fn description(mut self, description: impl Into>) -> Self { + /// Set the stream description. If this is not set, description provided while creating the instrument will be used. + pub fn with_description(mut self, description: impl Into>) -> Self { self.description = Some(description.into()); self } - /// Set the stream unit. - pub fn unit(mut self, unit: impl Into>) -> Self { + /// Set the stream unit. If this is not set, unit provided while creating the instrument will be used. + pub fn with_unit(mut self, unit: impl Into>) -> Self { self.unit = Some(unit.into()); self } #[cfg(feature = "spec_unstable_metrics_views")] - /// Set the stream aggregation. - pub fn aggregation(mut self, aggregation: Aggregation) -> Self { + /// Set the stream aggregation. This is used to customize the aggregation. + /// If not set, the default aggregation based on the instrument kind will be used. + pub fn with_aggregation(mut self, aggregation: Aggregation) -> Self { self.aggregation = Some(aggregation); self } @@ -242,18 +234,132 @@ impl Stream { /// Set the stream allowed attribute keys. /// /// Any attribute recorded for the stream with a key not in this set will be - /// dropped. If this set is empty all attributes will be dropped. - pub fn allowed_attribute_keys(mut self, attribute_keys: impl IntoIterator) -> Self { + /// dropped. If the set is empty, all attributes will be dropped, if `None` all + /// attributes will be kept. + pub fn with_allowed_attribute_keys( + mut self, + attribute_keys: impl IntoIterator, + ) -> Self { self.allowed_attribute_keys = Some(Arc::new(attribute_keys.into_iter().collect())); - self } - /// Set the stream cardinality limit. - pub fn cardinality_limit(mut self, limit: usize) -> Self { + /// Set the stream cardinality limit. If this is not set, the default limit of 2000 will be used. + pub fn with_cardinality_limit(mut self, limit: usize) -> Self { self.cardinality_limit = Some(limit); self } + + /// Build a new Stream instance using the configuration in this builder. + /// + /// # Returns + /// + /// A Result containing the new Stream instance or an error if the build failed. + pub fn build(self) -> Result> { + // TODO: Add same validation as already done while + // creating instruments. It is better to move validation logic + // to a common helper and call it from both places. + // The current implementations does a basic validation + // only to close the overall API design. + + // if name is provided, it must not be empty + if let Some(name) = &self.name { + if name.is_empty() { + return Err("Stream name must not be empty".into()); + } + } + + // if cardinality limit is provided, it must be greater than 0 + if let Some(limit) = self.cardinality_limit { + if limit == 0 { + return Err("Cardinality limit must be greater than 0".into()); + } + } + + // If the aggregation is set to Histogram, validate the bucket boundaries. + if let Some(aggregation) = &self.aggregation { + if let Aggregation::ExplicitBucketHistogram { boundaries, .. } = aggregation { + validate_bucket_boundaries(boundaries)?; + } + } + + Ok(Stream { + name: self.name, + description: self.description, + unit: self.unit, + aggregation: self.aggregation, + allowed_attribute_keys: self.allowed_attribute_keys, + cardinality_limit: self.cardinality_limit, + }) + } +} + +fn validate_bucket_boundaries(boundaries: &[f64]) -> Result<(), String> { + // Validate boundaries do not contain f64::NAN, f64::INFINITY, or f64::NEG_INFINITY + for boundary in boundaries { + if boundary.is_nan() || boundary.is_infinite() { + return Err( + "Bucket boundaries must not contain NaN, Infinity, or -Infinity".to_string(), + ); + } + } + + // validate that buckets are sorted and non-duplicate + for i in 1..boundaries.len() { + if boundaries[i] <= boundaries[i - 1] { + return Err("Bucket boundaries must be sorted and non-duplicate".to_string()); + } + } + + Ok(()) +} + +/// Describes the stream of data an instrument produces. +/// +/// # Example +/// +/// Streams can be used as masks in views. +/// +/// ``` +/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, StreamBuilder}; +/// +/// let criteria = Instrument::new().name("counter_*"); +/// let mask = StreamBuilder::new() +/// .with_aggregation(Aggregation::Sum) +/// .build() +/// .unwrap(); +/// +/// let view = new_view(criteria, mask); +/// # drop(view); +/// ``` +#[derive(Default, Debug)] +#[non_exhaustive] +#[allow(unreachable_pub)] +pub struct Stream { + /// The human-readable identifier of the stream. + pub(crate) name: Option>, + /// Describes the purpose of the data. + pub(crate) description: Option>, + /// the unit of measurement recorded. + pub(crate) unit: Option>, + /// Aggregation the stream uses for an instrument. + pub(crate) aggregation: Option, + /// An allow-list of attribute keys that will be preserved for the stream. + /// + /// Any attribute recorded for the stream with a key not in this set will be + /// dropped. If the set is empty, all attributes will be dropped, if `None` all + /// attributes will be kept. + pub(crate) allowed_attribute_keys: Option>>, + + /// Cardinality limit for the stream. + pub(crate) cardinality_limit: Option, +} + +impl Stream { + /// Create a new stream builder with default values. + pub fn builder() -> StreamBuilder { + StreamBuilder::new() + } } /// The identifying properties of an instrument. diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index cc6e85f0d2..73cc9c23d1 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -287,13 +287,83 @@ impl MeterProviderBuilder { self } - /// Associates a [View] with a [MeterProvider]. + /// Adds a [View] to the [MeterProvider]. /// - /// [View]s are appended to existing ones in a [MeterProvider] if this option is - /// used multiple times. + /// Views allow you to customize how metrics are aggregated, renamed, or + /// otherwise transformed before export, without modifying instrument + /// definitions in your application or library code. /// - /// By default, if this option is not used, the [MeterProvider] will use the - /// default view. + /// You can pass any type implementing the [`View`] trait, including + /// closures matching the pattern `Fn(&Instrument) -> Option`. The + /// function receives a reference to the [`Instrument`] and can return an + /// [`Option`] of [`Stream`] to specify how matching instruments should be + /// exported. Returning `None` means the view does not apply to the given + /// instrument, and the default behavior will be used. + /// + /// + /// # Examples + /// + /// Renaming a metric: + /// + /// ``` + /// # use opentelemetry_sdk::metrics::{Stream, Instrument}; + /// let view_rename = |i: &Instrument| { + /// if i.name == "my_counter" { + /// Some(Stream::builder().with_name("my_counter_renamed").build().expect("Stream should be valid")) + /// } else { + /// None + /// } + /// }; + /// # let builder = opentelemetry_sdk::metrics::SdkMeterProvider::builder(); + /// # let _builder = + /// builder.with_view(view_rename); + /// ``` + /// + /// Setting a cardinality limit to control resource usage: + /// + /// ``` + /// # use opentelemetry_sdk::metrics::{Stream, Instrument}; + /// let view_change_cardinality = |i: &Instrument| { + /// if i.name == "my_counter" { + /// Some( + /// Stream::builder() + /// .with_cardinality_limit(100).build().expect("Stream should be valid"), + /// ) + /// } else { + /// None + /// } + /// }; + /// # let builder = opentelemetry_sdk::metrics::SdkMeterProvider::builder(); + /// # let _builder = + /// builder.with_view(view_change_cardinality); + /// ``` + /// + /// Silently ignoring Stream build errors: + /// + /// ``` + /// # use opentelemetry_sdk::metrics::{Stream, Instrument}; + /// let my_view_change_cardinality = |i: &Instrument| { + /// if i.name == "my_second_histogram" { + /// // Note: If Stream is invalid, build() will return `Error` variant. + /// // By calling `.ok()`, any such error is ignored and treated as if the view does not match + /// // the instrument. + /// // If this is not the desired behavior, consider handling the error explicitly. + /// Stream::builder().with_cardinality_limit(100).build().ok() + /// } else { + /// None + /// } + /// }; + /// # let builder = opentelemetry_sdk::metrics::SdkMeterProvider::builder(); + /// # let _builder = + /// builder.with_view(my_view_change_cardinality); + /// ``` + /// + /// If no views are added, the [MeterProvider] uses the default view. + /// + /// [`Instrument`]: crate::metrics::Instrument + /// [`View`]: crate::metrics::View + /// [`Stream`]: crate::metrics::Stream + /// [`Option`]: core::option::Option pub fn with_view(mut self, view: T) -> Self { self.views.push(Arc::new(view)); self diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index e193481bdc..fc20f88f19 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -82,10 +82,12 @@ pub use periodic_reader::*; #[cfg(feature = "experimental_metrics_custom_reader")] pub use pipeline::Pipeline; -pub use instrument::{Instrument, InstrumentKind, Stream}; +pub use instrument::{Instrument, InstrumentKind, Stream, StreamBuilder}; #[cfg(feature = "spec_unstable_metrics_views")] -pub use view::*; +pub use view::new_view; + +pub use view::View; use std::hash::Hash; @@ -879,8 +881,8 @@ mod tests { assert_eq!(datapoint.value, 15); } + #[cfg(feature = "spec_unstable_metrics_views")] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[ignore = "TODO: This test should be fixed when Stream is moved to proper builder pattern."] async fn histogram_aggregation_with_invalid_aggregation_should_proceed_as_if_view_not_exist() { // Run this test with stdout enabled to see output. // cargo test histogram_aggregation_with_invalid_aggregation_should_proceed_as_if_view_not_exist --features=testing -- --nocapture @@ -889,15 +891,15 @@ mod tests { let exporter = InMemoryMetricExporter::default(); let view = |i: &Instrument| { if i.name == "test_histogram" { - Some( - Stream::new() - .aggregation(aggregation::Aggregation::ExplicitBucketHistogram { - boundaries: vec![0.9, 1.9, 1.2, 1.3, 1.4, 1.5], // invalid boundaries - record_min_max: false, - }) - .name("test_histogram_renamed") - .unit("test_unit_renamed"), - ) + Stream::builder() + .with_aggregation(aggregation::Aggregation::ExplicitBucketHistogram { + boundaries: vec![0.9, 1.9, 1.2, 1.3, 1.4, 1.5], // invalid boundaries + record_min_max: false, + }) + .with_name("test_histogram_renamed") + .with_unit("test_unit_renamed") + .build() + .ok() } else { None } @@ -933,6 +935,7 @@ mod tests { ); } + #[cfg(feature = "spec_unstable_metrics_views")] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[ignore = "Spatial aggregation is not yet implemented."] async fn spatial_aggregation_when_view_drops_attributes_observable_counter() { @@ -943,7 +946,12 @@ mod tests { // View drops all attributes. let view = |i: &Instrument| { if i.name == "my_observable_counter" { - Some(Stream::new().allowed_attribute_keys(vec![])) + Some( + Stream::builder() + .with_allowed_attribute_keys(vec![]) + .build() + .unwrap(), + ) } else { None } @@ -1010,6 +1018,7 @@ mod tests { assert_eq!(data_point.value, 300); } + #[cfg(feature = "spec_unstable_metrics_views")] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn spatial_aggregation_when_view_drops_attributes_counter() { // cargo test spatial_aggregation_when_view_drops_attributes_counter --features=testing @@ -1019,7 +1028,12 @@ mod tests { // View drops all attributes. let view = |i: &Instrument| { if i.name == "my_counter" { - Some(Stream::new().allowed_attribute_keys(vec![])) + Some( + Stream::builder() + .with_allowed_attribute_keys(vec![]) + .build() + .unwrap(), + ) } else { None } @@ -1474,7 +1488,12 @@ mod tests { test_view_customization( |i| { if i.name == "my_counter" { - Some(Stream::new().name("my_counter_renamed")) + Some( + Stream::builder() + .with_name("my_counter_renamed") + .build() + .unwrap(), + ) } else { None } @@ -1490,7 +1509,7 @@ mod tests { test_view_customization( |i| { if i.name == "my_counter" { - Some(Stream::new().unit("my_unit_new")) + Some(Stream::builder().with_unit("my_unit_new").build().unwrap()) } else { None } @@ -1506,7 +1525,12 @@ mod tests { test_view_customization( |i| { if i.name == "my_counter" { - Some(Stream::new().description("my_description_new")) + Some( + Stream::builder() + .with_description("my_description_new") + .build() + .unwrap(), + ) } else { None } @@ -1522,7 +1546,13 @@ mod tests { test_view_customization( |i| { if i.name == "my_counter" { - Some(Stream::new().name("my_counter_renamed").unit("my_unit_new")) + Some( + Stream::builder() + .with_name("my_counter_renamed") + .with_unit("my_unit_new") + .build() + .unwrap(), + ) } else { None } @@ -1539,10 +1569,12 @@ mod tests { |i| { if i.name == "my_counter" { Some( - Stream::new() - .name("my_counter_renamed") - .unit("my_unit_new") - .description("my_description_new"), + Stream::builder() + .with_name("my_counter_renamed") + .with_unit("my_unit_new") + .with_description("my_description_new") + .build() + .unwrap(), ) } else { None @@ -1559,7 +1591,7 @@ mod tests { test_view_customization( |i| { if i.unit == "my_unit" { - Some(Stream::new().unit("my_unit_new")) + Some(Stream::builder().with_unit("my_unit_new").build().unwrap()) } else { None } @@ -1575,7 +1607,7 @@ mod tests { test_view_customization( |i| { if i.name == "not_expected_to_match" { - Some(Stream::new()) + Some(Stream::builder().build().unwrap()) } else { None } @@ -1591,7 +1623,12 @@ mod tests { test_view_customization( |i| { if i.name == "my_counter" && i.unit == "my_unit" { - Some(Stream::new().name("my_counter_renamed")) + Some( + Stream::builder() + .with_name("my_counter_renamed") + .build() + .unwrap(), + ) } else { None } @@ -2726,9 +2763,11 @@ mod tests { let view_change_cardinality = move |i: &Instrument| { if i.name == "my_counter" { Some( - Stream::new() - .name("my_counter") - .cardinality_limit(cardinality_limit), + Stream::builder() + .with_name("my_counter") + .with_cardinality_limit(cardinality_limit) + .build() + .unwrap(), ) } else { None diff --git a/opentelemetry-sdk/src/metrics/view.rs b/opentelemetry-sdk/src/metrics/view.rs index a3ab2a4577..782ee364ae 100644 --- a/opentelemetry-sdk/src/metrics/view.rs +++ b/opentelemetry-sdk/src/metrics/view.rs @@ -93,10 +93,13 @@ impl View for Box { /// # Example /// /// ``` -/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, Stream}; +/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, StreamBuilder}; /// /// let criteria = Instrument::new().name("counter_*"); -/// let mask = Stream::new().aggregation(Aggregation::Sum); +/// let mask = Stream::builder() +/// .with_aggregation(Aggregation::Sum) +/// .build() +/// .unwrap(); /// /// let view = new_view(criteria, mask); /// # drop(view); @@ -175,7 +178,7 @@ mod tests { #[test] fn test_new_view_matching_all() { let criteria = Instrument::new().name("*"); - let mask = Stream::new(); + let mask = Stream::builder().build().unwrap(); let view = new_view(criteria, mask).expect("Expected to create a new view"); @@ -189,7 +192,7 @@ mod tests { #[test] fn test_new_view_exact_match() { let criteria = Instrument::new().name("counter_exact_match"); - let mask = Stream::new(); + let mask = Stream::builder().build().unwrap(); let view = new_view(criteria, mask).expect("Expected to create a new view"); @@ -209,7 +212,7 @@ mod tests { #[test] fn test_new_view_with_wildcard_pattern() { let criteria = Instrument::new().name("prefix_*"); - let mask = Stream::new(); + let mask = Stream::builder().build().unwrap(); let view = new_view(criteria, mask).expect("Expected to create a new view"); @@ -229,7 +232,7 @@ mod tests { #[test] fn test_new_view_wildcard_question_mark() { let criteria = Instrument::new().name("test_?"); - let mask = Stream::new(); + let mask = Stream::builder().build().unwrap(); let view = new_view(criteria, mask).expect("Expected to create a new view"); From 127eac60549956436b0918086bbfe864769b2926 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 20 May 2025 23:07:12 -0700 Subject: [PATCH 2/5] bench --- examples/metrics-advanced/src/main.rs | 2 +- opentelemetry-sdk/benches/metric.rs | 16 +++++++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index 00b8044698..bc644b73b0 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -26,7 +26,7 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { // Note: If Stream is invalid, build() will return an error. // By calling `.ok()`, any such error is ignored and treated as if the view does not exist. // Consider handling the error explicitly if this is not the desired behavior. - Stream::builder().with_cardinality_limit(0).build().ok() + Stream::builder().with_cardinality_limit(2).build().ok() } else { None } diff --git a/opentelemetry-sdk/benches/metric.rs b/opentelemetry-sdk/benches/metric.rs index 73c4142420..54eceb2116 100644 --- a/opentelemetry-sdk/benches/metric.rs +++ b/opentelemetry-sdk/benches/metric.rs @@ -223,7 +223,10 @@ fn counters(c: &mut Criterion) { Some( new_view( Instrument::new().name("*"), - Stream::new().allowed_attribute_keys([Key::new("K")]), + Stream::builder() + .with_allowed_attribute_keys([Key::new("K")]) + .build() + .unwrap(), ) .unwrap(), ), @@ -273,10 +276,13 @@ fn bench_histogram(bound_count: usize) -> (SharedReader, Histogram) { let view = Some( new_view( Instrument::new().name("histogram_*"), - Stream::new().aggregation(Aggregation::ExplicitBucketHistogram { - boundaries: bounds.iter().map(|&x| x as f64).collect(), - record_min_max: true, - }), + Stream::builder() + .with_aggregation(Aggregation::ExplicitBucketHistogram { + boundaries: bounds.iter().map(|&x| x as f64).collect(), + record_min_max: true, + }) + .build() + .unwrap(), ) .unwrap(), ); From c352e84ec75e934352b58351afe52b718c26be84 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 20 May 2025 23:08:34 -0700 Subject: [PATCH 3/5] cl --- opentelemetry-sdk/src/metrics/instrument.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/instrument.rs b/opentelemetry-sdk/src/metrics/instrument.rs index ffb7b320ee..ee18d82fe1 100644 --- a/opentelemetry-sdk/src/metrics/instrument.rs +++ b/opentelemetry-sdk/src/metrics/instrument.rs @@ -276,11 +276,9 @@ impl StreamBuilder { } } - // If the aggregation is set to Histogram, validate the bucket boundaries. - if let Some(aggregation) = &self.aggregation { - if let Aggregation::ExplicitBucketHistogram { boundaries, .. } = aggregation { - validate_bucket_boundaries(boundaries)?; - } + // If the aggregation is set to ExplicitBucketHistogram, validate the bucket boundaries. + if let Some(Aggregation::ExplicitBucketHistogram { boundaries, .. }) = &self.aggregation { + validate_bucket_boundaries(boundaries)?; } Ok(Stream { From 4bc32b6eb546c9d7b9d1022840a60fa296c0aebe Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 20 May 2025 23:11:59 -0700 Subject: [PATCH 4/5] nit wording --- examples/metrics-advanced/src/main.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index bc644b73b0..287ec2ca3b 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -23,9 +23,10 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { // for example 2 let my_view_change_cardinality = |i: &Instrument| { if i.name == "my_second_histogram" { - // Note: If Stream is invalid, build() will return an error. - // By calling `.ok()`, any such error is ignored and treated as if the view does not exist. - // Consider handling the error explicitly if this is not the desired behavior. + // Note: If Stream is invalid, build() will return an error. By + // calling `.ok()`, any such error is ignored and treated as if the + // view does not match the instrument. If this is not the desired + // behavior, consider handling the error explicitly. Stream::builder().with_cardinality_limit(2).build().ok() } else { None From f0636b5f2f1a0adab85f8aa14a026a32b0f32bbd Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Tue, 20 May 2025 23:22:19 -0700 Subject: [PATCH 5/5] doc fix --- opentelemetry-sdk/src/metrics/instrument.rs | 10 +++++----- opentelemetry-sdk/src/metrics/view.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/instrument.rs b/opentelemetry-sdk/src/metrics/instrument.rs index ee18d82fe1..259460ef67 100644 --- a/opentelemetry-sdk/src/metrics/instrument.rs +++ b/opentelemetry-sdk/src/metrics/instrument.rs @@ -73,7 +73,7 @@ impl InstrumentKind { /// Instruments can be used as criteria for views. /// /// ``` -/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, StreamBuilder}; +/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, Stream, StreamBuilder}; /// /// let criteria = Instrument::new().name("counter_*"); /// let mask = Stream::builder() @@ -177,10 +177,10 @@ impl Instrument { /// # Example /// /// ``` -/// use opentelemetry_sdk::metrics::{Aggregation, StreamBuilder}; +/// use opentelemetry_sdk::metrics::{Aggregation, Stream}; /// use opentelemetry::Key; /// -/// let stream = StreamBuilder::new() +/// let stream = Stream::builder() /// .with_name("my_stream") /// .with_aggregation(Aggregation::Sum) /// .with_cardinality_limit(100) @@ -319,10 +319,10 @@ fn validate_bucket_boundaries(boundaries: &[f64]) -> Result<(), String> { /// Streams can be used as masks in views. /// /// ``` -/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, StreamBuilder}; +/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, Stream}; /// /// let criteria = Instrument::new().name("counter_*"); -/// let mask = StreamBuilder::new() +/// let mask = Stream::builder() /// .with_aggregation(Aggregation::Sum) /// .build() /// .unwrap(); diff --git a/opentelemetry-sdk/src/metrics/view.rs b/opentelemetry-sdk/src/metrics/view.rs index 782ee364ae..ac2e0493d9 100644 --- a/opentelemetry-sdk/src/metrics/view.rs +++ b/opentelemetry-sdk/src/metrics/view.rs @@ -93,7 +93,7 @@ impl View for Box { /// # Example /// /// ``` -/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, StreamBuilder}; +/// use opentelemetry_sdk::metrics::{new_view, Aggregation, Instrument, Stream}; /// /// let criteria = Instrument::new().name("counter_*"); /// let mask = Stream::builder()