From 4f7ae7a9801712267d4c5e048a9f4b57ad23006e Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Mon, 4 Mar 2024 15:51:44 -0800 Subject: [PATCH 1/3] Add test to confirm bug in metrics sdk --- opentelemetry-sdk/src/metrics/mod.rs | 107 +++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index 96e35aeba4..1bce12d11f 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -590,6 +590,113 @@ mod tests { ); } + // "multi_thread" tokio flavor must be used else flush won't + // be able to make progress! + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + #[ignore = "Bug bug."] + async fn delta_memory_efficiency_test() { + // Run this test with stdout enabled to see output. + // cargo test delta_memory_efficiency_test --features=metrics,testing -- --nocapture + + // Arrange + let exporter = InMemoryMetricsExporterBuilder::new().with_temporality_selector(DeltaTemporalitySelector()).build(); + let reader = PeriodicReader::builder(exporter.clone(), runtime::Tokio).build(); + let meter_provider = SdkMeterProvider::builder().with_reader(reader).build(); + + // Act + let meter = meter_provider.meter("test"); + let counter = meter + .u64_counter("my_counter") + .with_unit(Unit::new("my_unit")) + .init(); + counter.add(1, &[KeyValue::new("key1", "value1")]); + counter.add(1, &[KeyValue::new("key1", "value1")]); + counter.add(1, &[KeyValue::new("key1", "value1")]); + counter.add(1, &[KeyValue::new("key1", "value1")]); + counter.add(1, &[KeyValue::new("key1", "value1")]); + + counter.add(1, &[KeyValue::new("key1", "value2")]); + counter.add(1, &[KeyValue::new("key1", "value2")]); + counter.add(1, &[KeyValue::new("key1", "value2")]); + + meter_provider.force_flush().unwrap(); + + // Assert + let resource_metrics = exporter + .get_finished_metrics() + .expect("metrics are expected to be exported."); + assert!(!resource_metrics.is_empty()); + let metric = &resource_metrics[0].scope_metrics[0].metrics[0]; + assert_eq!(metric.name, "my_counter"); + assert_eq!(metric.unit.as_str(), "my_unit"); + let sum = metric + .data + .as_any() + .downcast_ref::>() + .expect("Sum aggregation expected for Counter instruments by default"); + + // Expecting 2 time-series. + assert_eq!(sum.data_points.len(), 2); + assert!(sum.is_monotonic, "Counter should produce monotonic."); + assert_eq!( + sum.temporality, + data::Temporality::Delta, + "Should produce Delta as configured" + ); + + // find and validate key1=value1 datapoint + let mut data_point1 = None; + for datapoint in &sum.data_points { + if datapoint + .attributes + .iter() + .any(|(k, v)| k.as_str() == "key1" && v.as_str() == "value1") + { + data_point1 = Some(datapoint); + } + } + assert_eq!( + data_point1 + .expect("datapoint with key1=value1 expected") + .value, + 5 + ); + + // find and validate key1=value2 datapoint + let mut data_point1 = None; + for datapoint in &sum.data_points { + if datapoint + .attributes + .iter() + .any(|(k, v)| k.as_str() == "key1" && v.as_str() == "value2") + { + data_point1 = Some(datapoint); + } + } + assert_eq!( + data_point1 + .expect("datapoint with key1=value2 expected") + .value, + 3 + ); + + // flush again, and validate that nothing is flushed + // as delta temporality. + meter_provider.force_flush().unwrap(); + let resource_metrics = exporter + .get_finished_metrics() + .expect("metrics are expected to be exported."); + println!("resource_metrics: {:?}", resource_metrics); + assert!(resource_metrics.is_empty(), "No metrics should be exported as no new measurements were recorded since last collect."); + } + + struct DeltaTemporalitySelector(); + impl TemporalitySelector for DeltaTemporalitySelector { + fn temporality(&self, _kind: InstrumentKind) -> Temporality { + Temporality::Delta + } + } + struct TestContext { exporter: InMemoryMetricsExporter, meter_provider: SdkMeterProvider, From 8a55f14e1e460f5fce7ada90c186e09a8ed2e63c Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Mon, 4 Mar 2024 15:56:01 -0800 Subject: [PATCH 2/3] fmt fix --- opentelemetry-sdk/src/metrics/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index 1bce12d11f..66f6cf844f 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -599,7 +599,9 @@ mod tests { // cargo test delta_memory_efficiency_test --features=metrics,testing -- --nocapture // Arrange - let exporter = InMemoryMetricsExporterBuilder::new().with_temporality_selector(DeltaTemporalitySelector()).build(); + let exporter = InMemoryMetricsExporterBuilder::new() + .with_temporality_selector(DeltaTemporalitySelector()) + .build(); let reader = PeriodicReader::builder(exporter.clone(), runtime::Tokio).build(); let meter_provider = SdkMeterProvider::builder().with_reader(reader).build(); @@ -687,7 +689,7 @@ mod tests { .get_finished_metrics() .expect("metrics are expected to be exported."); println!("resource_metrics: {:?}", resource_metrics); - assert!(resource_metrics.is_empty(), "No metrics should be exported as no new measurements were recorded since last collect."); + assert!(resource_metrics.is_empty(), "No metrics should be exported as no new measurements were recorded since last collect."); } struct DeltaTemporalitySelector(); From acb4d173bf6719c27b13225545929f2d5b19baa5 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Mon, 4 Mar 2024 21:15:03 -0800 Subject: [PATCH 3/3] add bug id --- opentelemetry-sdk/src/metrics/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index 66f6cf844f..7d41cdf18d 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -593,7 +593,7 @@ mod tests { // "multi_thread" tokio flavor must be used else flush won't // be able to make progress! #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - #[ignore = "Bug bug."] + #[ignore = "Known bug: https://github.com/open-telemetry/opentelemetry-rust/issues/1598"] async fn delta_memory_efficiency_test() { // Run this test with stdout enabled to see output. // cargo test delta_memory_efficiency_test --features=metrics,testing -- --nocapture