From 2f65b5406f0f78c67da97d1fc1f820392082fd75 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 24 Oct 2024 10:05:16 -0700 Subject: [PATCH 1/5] Global error handler cleanup - ManualReader (#2236) Co-authored-by: Cijo Thomas --- opentelemetry-sdk/src/metrics/manual_reader.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/manual_reader.rs b/opentelemetry-sdk/src/metrics/manual_reader.rs index e1cb571ec3..b79b1d7ae7 100644 --- a/opentelemetry-sdk/src/metrics/manual_reader.rs +++ b/opentelemetry-sdk/src/metrics/manual_reader.rs @@ -4,8 +4,8 @@ use std::{ }; use opentelemetry::{ - global, metrics::{MetricsError, Result}, + otel_debug, }; use super::{ @@ -77,9 +77,9 @@ impl MetricReader for ManualReader { if inner.sdk_producer.is_none() { inner.sdk_producer = Some(pipeline); } else { - global::handle_error(MetricsError::Config( - "duplicate reader registration, did not register manual reader".into(), - )) + otel_debug!( + name: "ManualReader.DuplicateRegistration", + message = "The pipeline is already registered to the Reader. Registering pipeline multiple times is not allowed."); } }); } From 0202299dd71a6086ee4f3938c6ced7cee64adb40 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 24 Oct 2024 13:04:26 -0700 Subject: [PATCH 2/5] Pin pin_project_lite to older version due to external_type_check compatibility (#2239) --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index a91ecbb4e0..e08de8187d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,7 @@ hyper-util = "0.1" log = "0.4.21" once_cell = "1.13" ordered-float = "4.0" -pin-project-lite = "0.2" +pin-project-lite = "=0.2.14" # 0.2.15 is failing for cargo-check-external-types prost = "0.13" prost-build = "0.13" prost-types = "0.13" From d3b1c47e1aa3b96edaec0297ccb86ec7668bc0c0 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 24 Oct 2024 13:41:41 -0700 Subject: [PATCH 3/5] Global error handler cleanup - MeterProvider (#2237) --- .../src/metrics/meter_provider.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 591be34165..c91773434b 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -8,9 +8,8 @@ use std::{ }; use opentelemetry::{ - global, metrics::{Meter, MeterProvider, MetricsError, Result}, - KeyValue, + otel_debug, otel_error, KeyValue, }; use crate::{instrumentation::Scope, Resource}; @@ -137,13 +136,21 @@ impl Drop for SdkMeterProviderInner { fn drop(&mut self) { // If user has already shutdown the provider manually by calling // shutdown(), then we don't need to call shutdown again. - if !self.is_shutdown.load(Ordering::Relaxed) { - if let Err(err) = self.shutdown() { - global::handle_error(err); - } + if self.is_shutdown.load(Ordering::Relaxed) { + otel_debug!( + name: "MeterProvider.AlreadyShutdown", + message = "Meter provider was already shut down; drop will not attempt shutdown again." + ); + } else if let Err(err) = self.shutdown() { + otel_error!( + name: "MeterProvider.ShutdownFailed", + message = "Shutdown attempt failed during drop of MeterProvider.", + reason = format!("{}", err) + ); } } } + impl MeterProvider for SdkMeterProvider { fn versioned_meter( &self, From a18853e47afa9afe0d9e176bd352bd1926f96486 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 24 Oct 2024 13:43:46 -0700 Subject: [PATCH 4/5] Global error handler cleanup - exponential histogram (#2235) Co-authored-by: Zhongyang Wu Co-authored-by: Cijo Thomas --- .../metrics/internal/exponential_histogram.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/internal/exponential_histogram.rs b/opentelemetry-sdk/src/metrics/internal/exponential_histogram.rs index c23b441663..e85e0ece55 100644 --- a/opentelemetry-sdk/src/metrics/internal/exponential_histogram.rs +++ b/opentelemetry-sdk/src/metrics/internal/exponential_histogram.rs @@ -1,7 +1,7 @@ use std::{collections::HashMap, f64::consts::LOG2_E, sync::Mutex, time::SystemTime}; use once_cell::sync::Lazy; -use opentelemetry::{metrics::MetricsError, KeyValue}; +use opentelemetry::{otel_debug, KeyValue}; use crate::{ metrics::data::{self, Aggregation, Temporality}, @@ -100,9 +100,18 @@ impl ExpoHistogramDataPoint { if (self.scale - scale_delta as i8) < EXPO_MIN_SCALE { // With a scale of -10 there is only two buckets for the whole range of f64 values. // This can only happen if there is a max size of 1. - opentelemetry::global::handle_error(MetricsError::Other( - "exponential histogram scale underflow".into(), - )); + + // TODO - to check if this should be logged as an error if this is auto-recoverable. + otel_debug!( + name: "ExponentialHistogramDataPoint.Scale.Underflow", + current_scale = self.scale, + scale_delta = scale_delta, + max_size = self.max_size, + min_scale = EXPO_MIN_SCALE, + value = format!("{:?}", v), + message = "The measurement will be dropped due to scale underflow. Check the histogram configuration" + ); + return; } // Downscale From 5628f66395e737d3085c6f4ab6ed4cffee8a4194 Mon Sep 17 00:00:00 2001 From: Jialun Cai Date: Fri, 25 Oct 2024 12:34:40 +0800 Subject: [PATCH 5/5] Exclude benchmark setup duration using iter_batched (#2233) --- opentelemetry-sdk/benches/metrics_counter.rs | 112 ++++++++++--------- 1 file changed, 61 insertions(+), 51 deletions(-) diff --git a/opentelemetry-sdk/benches/metrics_counter.rs b/opentelemetry-sdk/benches/metrics_counter.rs index 24a4403266..8361c423ef 100644 --- a/opentelemetry-sdk/benches/metrics_counter.rs +++ b/opentelemetry-sdk/benches/metrics_counter.rs @@ -12,7 +12,7 @@ | ThreadLocal_Random_Generator_5 | 37 ns | */ -use criterion::{criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, criterion_main, BatchSize, Criterion}; use opentelemetry::{ metrics::{Counter, MeterProvider as _}, KeyValue, @@ -57,62 +57,72 @@ fn criterion_benchmark(c: &mut Criterion) { fn counter_add_sorted(c: &mut Criterion) { let counter = create_counter("Counter_Add_Sorted"); c.bench_function("Counter_Add_Sorted", |b| { - b.iter(|| { - // 4*4*10*10 = 1600 time series. - let rands = CURRENT_RNG.with(|rng| { - let mut rng = rng.borrow_mut(); - [ - rng.gen_range(0..4), - rng.gen_range(0..4), - rng.gen_range(0..10), - rng.gen_range(0..10), - ] - }); - let index_first_attribute = rands[0]; - let index_second_attribute = rands[1]; - let index_third_attribute = rands[2]; - let index_fourth_attribute = rands[3]; - counter.add( - 1, - &[ - KeyValue::new("attribute1", ATTRIBUTE_VALUES[index_first_attribute]), - KeyValue::new("attribute2", ATTRIBUTE_VALUES[index_second_attribute]), - KeyValue::new("attribute3", ATTRIBUTE_VALUES[index_third_attribute]), - KeyValue::new("attribute4", ATTRIBUTE_VALUES[index_fourth_attribute]), - ], - ); - }); + b.iter_batched( + || { + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); + [ + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }) + }, + |rands| { + let index_first_attribute = rands[0]; + let index_second_attribute = rands[1]; + let index_third_attribute = rands[2]; + let index_fourth_attribute = rands[3]; + counter.add( + 1, + &[ + KeyValue::new("attribute1", ATTRIBUTE_VALUES[index_first_attribute]), + KeyValue::new("attribute2", ATTRIBUTE_VALUES[index_second_attribute]), + KeyValue::new("attribute3", ATTRIBUTE_VALUES[index_third_attribute]), + KeyValue::new("attribute4", ATTRIBUTE_VALUES[index_fourth_attribute]), + ], + ); + }, + BatchSize::SmallInput, + ); }); } fn counter_add_unsorted(c: &mut Criterion) { let counter = create_counter("Counter_Add_Unsorted"); c.bench_function("Counter_Add_Unsorted", |b| { - b.iter(|| { - // 4*4*10*10 = 1600 time series. - let rands = CURRENT_RNG.with(|rng| { - let mut rng = rng.borrow_mut(); - [ - rng.gen_range(0..4), - rng.gen_range(0..4), - rng.gen_range(0..10), - rng.gen_range(0..10), - ] - }); - let index_first_attribute = rands[0]; - let index_second_attribute = rands[1]; - let index_third_attribute = rands[2]; - let index_fourth_attribute = rands[3]; - counter.add( - 1, - &[ - KeyValue::new("attribute2", ATTRIBUTE_VALUES[index_second_attribute]), - KeyValue::new("attribute3", ATTRIBUTE_VALUES[index_third_attribute]), - KeyValue::new("attribute1", ATTRIBUTE_VALUES[index_first_attribute]), - KeyValue::new("attribute4", ATTRIBUTE_VALUES[index_fourth_attribute]), - ], - ); - }); + b.iter_batched( + || { + // 4*4*10*10 = 1600 time series. + CURRENT_RNG.with(|rng| { + let mut rng = rng.borrow_mut(); + [ + rng.gen_range(0..4), + rng.gen_range(0..4), + rng.gen_range(0..10), + rng.gen_range(0..10), + ] + }) + }, + |rands| { + let index_first_attribute = rands[0]; + let index_second_attribute = rands[1]; + let index_third_attribute = rands[2]; + let index_fourth_attribute = rands[3]; + counter.add( + 1, + &[ + KeyValue::new("attribute2", ATTRIBUTE_VALUES[index_second_attribute]), + KeyValue::new("attribute3", ATTRIBUTE_VALUES[index_third_attribute]), + KeyValue::new("attribute1", ATTRIBUTE_VALUES[index_first_attribute]), + KeyValue::new("attribute4", ATTRIBUTE_VALUES[index_fourth_attribute]), + ], + ); + }, + BatchSize::SmallInput, + ); }); }