diff --git a/examples/metrics-advanced/Cargo.toml b/examples/metrics-advanced/Cargo.toml index 734f890748..40a4573d68 100644 --- a/examples/metrics-advanced/Cargo.toml +++ b/examples/metrics-advanced/Cargo.toml @@ -13,6 +13,6 @@ bench = false [dependencies] opentelemetry = { path = "../../opentelemetry", features = ["metrics"] } -opentelemetry_sdk = { path = "../../opentelemetry-sdk", features = ["spec_unstable_metrics_views"] } +opentelemetry_sdk = { path = "../../opentelemetry-sdk" } opentelemetry-stdout = { workspace = true, features = ["metrics"] } tokio = { workspace = true, features = ["full"] } diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index 27150b522c..19c0cd0e89 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -1,7 +1,6 @@ use opentelemetry::global; -use opentelemetry::Key; use opentelemetry::KeyValue; -use opentelemetry_sdk::metrics::{Aggregation, Instrument, SdkMeterProvider, Stream, Temporality}; +use opentelemetry_sdk::metrics::{Instrument, SdkMeterProvider, Stream, Temporality}; use opentelemetry_sdk::Resource; use std::error::Error; @@ -20,23 +19,9 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { }; // for example 2 - let my_view_drop_attributes = |i: &Instrument| { - if i.name == "my_counter" { - Some(Stream::new().allowed_attribute_keys(vec![Key::from("mykey1")])) - } else { - None - } - }; - - // for example 3 - let my_view_change_aggregation = |i: &Instrument| { + let my_view_change_cardinality = |i: &Instrument| { if i.name == "my_second_histogram" { - Some( - Stream::new().aggregation(Aggregation::ExplicitBucketHistogram { - boundaries: vec![0.9, 1.0, 1.1, 1.2, 1.3, 1.4, 1.5], - record_min_max: false, - }), - ) + Some(Stream::new().cardinality_limit(2)) } else { None } @@ -55,8 +40,7 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { .with_periodic_exporter(exporter) .with_resource(resource) .with_view(my_view_rename_and_unit) - .with_view(my_view_drop_attributes) - .with_view(my_view_change_aggregation) + .with_view(my_view_change_cardinality) .build(); global::set_meter_provider(provider.clone()); provider @@ -88,27 +72,7 @@ async fn main() -> Result<(), Box> { ], ); - // Example 2 - Drop unwanted attributes using view. - let counter = meter.u64_counter("my_counter").build(); - - // Record measurements using the Counter instrument. - // Though we are passing 4 attributes here, only 1 will be used - // for aggregation as view is configured to use only "mykey1" - // attribute. - counter.add( - 10, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - KeyValue::new("mykey3", "myvalue3"), - KeyValue::new("mykey4", "myvalue4"), - ], - ); - - // Example 3 - Change Aggregation configuration using View. - // Histograms are by default aggregated using ExplicitBucketHistogram - // with default buckets. The configured view will change the aggregation to - // use a custom set of boundaries, and min/max values will not be recorded. + // Example 2 - Change cardinality using View. let histogram2 = meter .f64_histogram("my_second_histogram") .with_unit("ms") @@ -116,37 +80,23 @@ async fn main() -> Result<(), Box> { .build(); // Record measurements using the histogram instrument. - // The values recorded are in the range of 1.2 to 1.5, warranting - // the change of boundaries. - histogram2.record( - 1.5, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - KeyValue::new("mykey3", "myvalue3"), - KeyValue::new("mykey4", "myvalue4"), - ], - ); + // This metric will have a cardinality limit of 2, + // as set in the view. Because of this, only the first two + // measurements will be recorded, and the rest will be folded + // into the overflow attribute. + histogram2.record(1.5, &[KeyValue::new("mykey1", "v1")]); - histogram2.record( - 1.2, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - KeyValue::new("mykey3", "myvalue3"), - KeyValue::new("mykey4", "myvalue4"), - ], - ); + histogram2.record(1.2, &[KeyValue::new("mykey1", "v2")]); - histogram2.record( - 1.23, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - KeyValue::new("mykey3", "myvalue3"), - KeyValue::new("mykey4", "myvalue4"), - ], - ); + histogram2.record(1.23, &[KeyValue::new("mykey1", "v3")]); + + histogram2.record(1.4, &[KeyValue::new("mykey1", "v4")]); + + histogram2.record(1.6, &[KeyValue::new("mykey1", "v5")]); + + histogram2.record(1.7, &[KeyValue::new("mykey1", "v6")]); + + histogram2.record(1.8, &[KeyValue::new("mykey1", "v7")]); // Metrics are exported by default every 30 seconds when using stdout exporter, // however shutting down the MeterProvider here instantly flushes diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index c813c74625..a062bf23bf 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -43,8 +43,13 @@ also modified to suppress telemetry before invoking exporters. [#2928](https://github.com/open-telemetry/opentelemetry-rust/pull/2928) - TODO: Placeholder for View related changelog. Polish this after all - changes are done. - Hide public fields from `Stream` struct. + - The `Stream` struct now has its public fields hidden. + - 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. - *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/aggregation.rs b/opentelemetry-sdk/src/metrics/aggregation.rs index 4c0804f319..02bdb1e215 100644 --- a/opentelemetry-sdk/src/metrics/aggregation.rs +++ b/opentelemetry-sdk/src/metrics/aggregation.rs @@ -150,9 +150,9 @@ impl Aggregation { #[cfg(test)] mod tests { + use super::Aggregation; use crate::metrics::error::{MetricError, MetricResult}; use crate::metrics::internal::{EXPO_MAX_SCALE, EXPO_MIN_SCALE}; - use crate::metrics::Aggregation; #[test] fn validate_aggregation() { diff --git a/opentelemetry-sdk/src/metrics/instrument.rs b/opentelemetry-sdk/src/metrics/instrument.rs index 63464fa5f1..2fffbeca42 100644 --- a/opentelemetry-sdk/src/metrics/instrument.rs +++ b/opentelemetry-sdk/src/metrics/instrument.rs @@ -83,7 +83,6 @@ impl InstrumentKind { /// ``` #[derive(Clone, Default, Debug, PartialEq)] #[non_exhaustive] -#[allow(unreachable_pub)] pub struct Instrument { /// The human-readable identifier of the instrument. pub name: Cow<'static, str>, @@ -208,7 +207,6 @@ pub struct Stream { pub(crate) cardinality_limit: Option, } -#[cfg(feature = "spec_unstable_metrics_views")] impl Stream { /// Create a new stream with empty values. pub fn new() -> Self { @@ -233,12 +231,14 @@ impl Stream { self } + #[cfg(feature = "spec_unstable_metrics_views")] /// Set the stream aggregation. pub fn aggregation(mut self, aggregation: Aggregation) -> Self { self.aggregation = Some(aggregation); self } + #[cfg(feature = "spec_unstable_metrics_views")] /// Set the stream allowed attribute keys. /// /// Any attribute recorded for the stream with a key not in this set will be diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index c4d5f86c32..cc6e85f0d2 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -287,7 +287,6 @@ impl MeterProviderBuilder { self } - #[cfg(feature = "spec_unstable_metrics_views")] /// Associates a [View] with a [MeterProvider]. /// /// [View]s are appended to existing ones in a [MeterProvider] if this option is diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index c98d7affa8..e193481bdc 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -82,11 +82,7 @@ pub use periodic_reader::*; #[cfg(feature = "experimental_metrics_custom_reader")] pub use pipeline::Pipeline; -#[cfg(feature = "experimental_metrics_custom_reader")] -pub use instrument::InstrumentKind; - -#[cfg(feature = "spec_unstable_metrics_views")] -pub use instrument::*; +pub use instrument::{Instrument, InstrumentKind, Stream}; #[cfg(feature = "spec_unstable_metrics_views")] pub use view::*; @@ -121,6 +117,7 @@ mod tests { use self::data::{HistogramDataPoint, ScopeMetrics, SumDataPoint}; use super::data::MetricData; use super::internal::Number; + use super::view::View; use super::*; use crate::metrics::data::ResourceMetrics; use crate::metrics::internal::AggregatedMetricsAccess; @@ -883,23 +880,28 @@ mod tests { } #[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 // Arrange let exporter = InMemoryMetricExporter::default(); - let criteria = Instrument::new().name("test_histogram"); - let stream_invalid_aggregation = 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"); - - let view = - new_view(criteria, stream_invalid_aggregation).expect("Expected to create a new view"); + 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"), + ) + } else { + None + } + }; let meter_provider = SdkMeterProvider::builder() .with_periodic_exporter(exporter.clone()) .with_view(view) @@ -938,12 +940,14 @@ mod tests { // Arrange let exporter = InMemoryMetricExporter::default(); - let criteria = Instrument::new().name("my_observable_counter"); // View drops all attributes. - let stream_invalid_aggregation = Stream::new().allowed_attribute_keys(vec![]); - - let view = - new_view(criteria, stream_invalid_aggregation).expect("Expected to create a new view"); + let view = |i: &Instrument| { + if i.name == "my_observable_counter" { + Some(Stream::new().allowed_attribute_keys(vec![])) + } else { + None + } + }; let meter_provider = SdkMeterProvider::builder() .with_periodic_exporter(exporter.clone()) .with_view(view) @@ -1012,12 +1016,14 @@ mod tests { // Arrange let exporter = InMemoryMetricExporter::default(); - let criteria = Instrument::new().name("my_counter"); // View drops all attributes. - let stream_invalid_aggregation = Stream::new().allowed_attribute_keys(vec![]); - - let view = - new_view(criteria, stream_invalid_aggregation).expect("Expected to create a new view"); + let view = |i: &Instrument| { + if i.name == "my_counter" { + Some(Stream::new().allowed_attribute_keys(vec![])) + } else { + None + } + }; let meter_provider = SdkMeterProvider::builder() .with_periodic_exporter(exporter.clone()) .with_view(view) @@ -1463,8 +1469,8 @@ mod tests { ); } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_rename() { + #[test] + fn view_test_rename() { test_view_customization( |i| { if i.name == "my_counter" { @@ -1477,11 +1483,10 @@ mod tests { "my_unit", "my_description", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_change_unit() { + #[test] + fn view_test_change_unit() { test_view_customization( |i| { if i.name == "my_counter" { @@ -1494,11 +1499,10 @@ mod tests { "my_unit_new", "my_description", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_change_description() { + #[test] + fn view_test_change_description() { test_view_customization( |i| { if i.name == "my_counter" { @@ -1511,11 +1515,10 @@ mod tests { "my_unit", "my_description_new", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_change_name_unit() { + #[test] + fn view_test_change_name_unit() { test_view_customization( |i| { if i.name == "my_counter" { @@ -1528,11 +1531,10 @@ mod tests { "my_unit_new", "my_description", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_change_name_unit_desc() { + #[test] + fn view_test_change_name_unit_desc() { test_view_customization( |i| { if i.name == "my_counter" { @@ -1550,11 +1552,10 @@ mod tests { "my_unit_new", "my_description_new", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_match_unit() { + #[test] + fn view_test_match_unit() { test_view_customization( |i| { if i.unit == "my_unit" { @@ -1567,11 +1568,10 @@ mod tests { "my_unit_new", "my_description", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_match_none() { + #[test] + fn view_test_match_none() { test_view_customization( |i| { if i.name == "not_expected_to_match" { @@ -1584,11 +1584,10 @@ mod tests { "my_unit", "my_description", ) - .await; } - #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn view_test_match_multiple() { + #[test] + fn view_test_match_multiple() { test_view_customization( |i| { if i.name == "my_counter" && i.unit == "my_unit" { @@ -1601,11 +1600,10 @@ mod tests { "my_unit", "my_description", ) - .await; } /// Helper function to test view customizations - async fn test_view_customization( + fn test_view_customization( view_function: F, expected_name: &str, expected_unit: &str, diff --git a/opentelemetry-sdk/src/metrics/pipeline.rs b/opentelemetry-sdk/src/metrics/pipeline.rs index e4d62b3955..e012211ffc 100644 --- a/opentelemetry-sdk/src/metrics/pipeline.rs +++ b/opentelemetry-sdk/src/metrics/pipeline.rs @@ -363,6 +363,9 @@ where kind: InstrumentKind, mut stream: Stream, ) -> MetricResult>>> { + // TODO: Create a separate pub (crate) Stream struct for the pipeline, + // as Stream will not have any optional fields as None at this point and + // new struct can better reflect this. let mut agg = stream .aggregation .take() diff --git a/opentelemetry-sdk/src/metrics/view.rs b/opentelemetry-sdk/src/metrics/view.rs index eab2916664..a3ab2a4577 100644 --- a/opentelemetry-sdk/src/metrics/view.rs +++ b/opentelemetry-sdk/src/metrics/view.rs @@ -45,7 +45,6 @@ fn empty_view(_inst: &Instrument) -> Option { /// let provider = SdkMeterProvider::builder().with_view(my_view).build(); /// # drop(provider) /// ``` -#[allow(unreachable_pub)] pub trait View: Send + Sync + 'static { /// Defines how data should be collected for certain instruments. ///