From aca4b52e20ee6728c91eb74744adfffa68e061d6 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 23 May 2024 14:38:17 -0700 Subject: [PATCH 01/13] Remove temporary vector in tracer appender (#1814) --- .../benches/logs.rs | 2 +- opentelemetry-appender-tracing/src/layer.rs | 79 +++++++++---------- stress/src/logs.rs | 2 +- 3 files changed, 41 insertions(+), 42 deletions(-) diff --git a/opentelemetry-appender-tracing/benches/logs.rs b/opentelemetry-appender-tracing/benches/logs.rs index a65d4c2b3d..f7ef4d94ad 100644 --- a/opentelemetry-appender-tracing/benches/logs.rs +++ b/opentelemetry-appender-tracing/benches/logs.rs @@ -10,7 +10,7 @@ | noop_layer_disabled | 12 ns | | noop_layer_enabled | 25 ns | | ot_layer_disabled | 19 ns | - | ot_layer_enabled | 446 ns | + | ot_layer_enabled | 371 ns | */ use async_trait::async_trait; diff --git a/opentelemetry-appender-tracing/src/layer.rs b/opentelemetry-appender-tracing/src/layer.rs index fe28a786dc..72d9e276e9 100644 --- a/opentelemetry-appender-tracing/src/layer.rs +++ b/opentelemetry-appender-tracing/src/layer.rs @@ -11,10 +11,8 @@ use tracing_subscriber::Layer; const INSTRUMENTATION_LIBRARY_NAME: &str = "opentelemetry-appender-tracing"; /// Visitor to record the fields from the event record. -#[derive(Default)] -struct EventVisitor { - log_record_attributes: Vec<(Key, AnyValue)>, - log_record_body: Option, +struct EventVisitor<'a, LR: LogRecord> { + log_record: &'a mut LR, } /// Logs from the log crate have duplicated attributes that we removed here. @@ -37,10 +35,13 @@ fn get_filename(filepath: &str) -> &str { filepath } -impl EventVisitor { +impl<'a, LR: LogRecord> EventVisitor<'a, LR> { + fn new(log_record: &'a mut LR) -> Self { + EventVisitor { log_record } + } fn visit_metadata(&mut self, meta: &Metadata) { - self.log_record_attributes - .push(("name".into(), meta.name().into())); + self.log_record + .add_attribute(Key::new("name"), AnyValue::from(meta.name())); #[cfg(feature = "experimental_metadata_attributes")] self.visit_experimental_metadata(meta); @@ -48,48 +49,47 @@ impl EventVisitor { #[cfg(feature = "experimental_metadata_attributes")] fn visit_experimental_metadata(&mut self, meta: &Metadata) { - self.log_record_attributes - .push(("log.target".into(), meta.target().to_owned().into())); + self.log_record.add_attribute( + Key::new("log.target"), + AnyValue::from(meta.target().to_owned()), + ); if let Some(module_path) = meta.module_path() { - self.log_record_attributes - .push(("code.namespace".into(), module_path.to_owned().into())); + self.log_record.add_attribute( + Key::new("code.namespace"), + AnyValue::from(module_path.to_owned()), + ); } if let Some(filepath) = meta.file() { - self.log_record_attributes - .push(("code.filepath".into(), filepath.to_owned().into())); - self.log_record_attributes.push(( - "code.filename".into(), - get_filename(filepath).to_owned().into(), - )); + self.log_record.add_attribute( + Key::new("code.filepath"), + AnyValue::from(filepath.to_owned()), + ); + self.log_record.add_attribute( + Key::new("code.filename"), + AnyValue::from(get_filename(filepath).to_owned()), + ); } if let Some(line) = meta.line() { - self.log_record_attributes - .push(("code.lineno".into(), line.into())); - } - } - - fn push_to_otel_log_record(self, log_record: &mut LR) { - if let Some(body) = self.log_record_body { - log_record.set_body(body); + self.log_record + .add_attribute(Key::new("code.lineno"), AnyValue::from(line)); } - log_record.add_attributes(self.log_record_attributes); } } -impl tracing::field::Visit for EventVisitor { +impl<'a, LR: LogRecord> tracing::field::Visit for EventVisitor<'a, LR> { fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { #[cfg(feature = "experimental_metadata_attributes")] if is_duplicated_metadata(field.name()) { return; } if field.name() == "message" { - self.log_record_body = Some(format!("{value:?}").into()); + self.log_record.set_body(format!("{:?}", value).into()); } else { - self.log_record_attributes - .push((field.name().into(), format!("{value:?}").into())); + self.log_record + .add_attribute(Key::new(field.name()), AnyValue::from(format!("{value:?}"))); } } @@ -98,18 +98,18 @@ impl tracing::field::Visit for EventVisitor { if is_duplicated_metadata(field.name()) { return; } - self.log_record_attributes - .push((field.name().into(), value.to_owned().into())); + self.log_record + .add_attribute(Key::new(field.name()), AnyValue::from(value.to_owned())); } fn record_bool(&mut self, field: &tracing_core::Field, value: bool) { - self.log_record_attributes - .push((field.name().into(), value.into())); + self.log_record + .add_attribute(Key::new(field.name()), AnyValue::from(value)); } fn record_f64(&mut self, field: &tracing::field::Field, value: f64) { - self.log_record_attributes - .push((field.name().into(), value.into())); + self.log_record + .add_attribute(Key::new(field.name()), AnyValue::from(value)); } fn record_i64(&mut self, field: &tracing::field::Field, value: i64) { @@ -117,8 +117,8 @@ impl tracing::field::Visit for EventVisitor { if is_duplicated_metadata(field.name()) { return; } - self.log_record_attributes - .push((field.name().into(), value.into())); + self.log_record + .add_attribute(Key::new(field.name()), AnyValue::from(value)); } // TODO: Remaining field types from AnyValue : Bytes, ListAny, Boolean @@ -173,11 +173,10 @@ where log_record.set_severity_number(severity_of_level(meta.level())); log_record.set_severity_text(meta.level().to_string().into()); - let mut visitor = EventVisitor::default(); + let mut visitor = EventVisitor::new(&mut log_record); visitor.visit_metadata(meta); // Visit fields. event.record(&mut visitor); - visitor.push_to_otel_log_record(&mut log_record); self.logger.emit(log_record); } diff --git a/stress/src/logs.rs b/stress/src/logs.rs index a4d4d48ae7..e973cc71a4 100644 --- a/stress/src/logs.rs +++ b/stress/src/logs.rs @@ -3,7 +3,7 @@ OS: Ubuntu 22.04.3 LTS (5.15.146.1-microsoft-standard-WSL2) Hardware: AMD EPYC 7763 64-Core Processor - 2.44 GHz, 16vCPUs, RAM: 64.0 GB - 39 M/sec + 53 M/sec */ use opentelemetry_appender_tracing::layer; From ca03c79ee1bfd49b05b3da746807968716bc5219 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Thu, 23 May 2024 19:20:09 -0700 Subject: [PATCH 02/13] Add bench for gauge instrument (#1816) --- opentelemetry-sdk/Cargo.toml | 5 +- opentelemetry-sdk/benches/metric_gauge.rs | 82 +++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 opentelemetry-sdk/benches/metric_gauge.rs diff --git a/opentelemetry-sdk/Cargo.toml b/opentelemetry-sdk/Cargo.toml index b535dd5438..ea402f38d8 100644 --- a/opentelemetry-sdk/Cargo.toml +++ b/opentelemetry-sdk/Cargo.toml @@ -65,7 +65,10 @@ harness = false [[bench]] name = "metric_counter" harness = false -required-features = ["metrics"] + +[[bench]] +name = "metric_gauge" +harness = false [[bench]] name = "attribute_set" diff --git a/opentelemetry-sdk/benches/metric_gauge.rs b/opentelemetry-sdk/benches/metric_gauge.rs new file mode 100644 index 0000000000..3217480760 --- /dev/null +++ b/opentelemetry-sdk/benches/metric_gauge.rs @@ -0,0 +1,82 @@ +/* + The benchmark results: + criterion = "0.5.1" + OS: Ubuntu 22.04.3 LTS (5.15.146.1-microsoft-standard-WSL2) + Hardware: AMD EPYC 7763 64-Core Processor - 2.44 GHz, 16vCPUs, + RAM: 64.0 GB + | Test | Average time| + |--------------------------------|-------------| + | Gauge_Add_4 | 586 ns | +*/ + +use criterion::{criterion_group, criterion_main, Criterion}; +use opentelemetry::{ + metrics::{Gauge, MeterProvider as _}, + KeyValue, +}; +use opentelemetry_sdk::metrics::{ManualReader, SdkMeterProvider}; +use rand::{ + rngs::{self}, + Rng, SeedableRng, +}; +use std::cell::RefCell; + +thread_local! { + /// Store random number generator for each thread + static CURRENT_RNG: RefCell = RefCell::new(rngs::SmallRng::from_entropy()); +} + +// Run this benchmark with: +// cargo bench --bench metric_gauge +fn create_gauge() -> Gauge { + let meter_provider: SdkMeterProvider = SdkMeterProvider::builder() + .with_reader(ManualReader::builder().build()) + .build(); + let meter = meter_provider.meter("benchmarks"); + + meter.u64_gauge("gauge_bench").init() +} + +fn criterion_benchmark(c: &mut Criterion) { + gauge_record(c); +} + +fn gauge_record(c: &mut Criterion) { + let attribute_values = [ + "value1", "value2", "value3", "value4", "value5", "value6", "value7", "value8", "value9", + "value10", + ]; + + let gauge = create_gauge(); + c.bench_function("Gauge_Add_4", |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_forth_attribute = rands[3]; + gauge.record( + 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_forth_attribute]), + ], + ); + }); + }); +} + +criterion_group!(benches, criterion_benchmark); + +criterion_main!(benches); From bded5989ef21dac2217629939d79e1e190f05ec1 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Thu, 23 May 2024 20:43:18 -0700 Subject: [PATCH 03/13] Fix overflow for counter metric (#1815) Co-authored-by: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> --- opentelemetry-sdk/benches/metric_counter.rs | 37 ++++++++++++---- .../src/metrics/internal/aggregate.rs | 2 +- opentelemetry-sdk/src/metrics/internal/sum.rs | 10 ++--- opentelemetry-sdk/src/metrics/mod.rs | 29 +++++++++++++ stress/Cargo.toml | 5 +++ stress/src/metrics_overflow.rs | 43 +++++++++++++++++++ 6 files changed, 112 insertions(+), 14 deletions(-) create mode 100644 stress/src/metrics_overflow.rs diff --git a/opentelemetry-sdk/benches/metric_counter.rs b/opentelemetry-sdk/benches/metric_counter.rs index d6e42bb6d5..8962daa125 100644 --- a/opentelemetry-sdk/benches/metric_counter.rs +++ b/opentelemetry-sdk/benches/metric_counter.rs @@ -19,7 +19,7 @@ use opentelemetry::{ }; use opentelemetry_sdk::metrics::{ManualReader, SdkMeterProvider}; use rand::{ - rngs::{self, SmallRng}, + rngs::{self}, Rng, SeedableRng, }; use std::cell::RefCell; @@ -107,14 +107,35 @@ fn counter_add(c: &mut Criterion) { }); }); - c.bench_function("Random_Generator_5", |b| { + // Cause overflow. + for v in 0..2001 { + counter.add(100, &[KeyValue::new("A", v.to_string())]); + } + c.bench_function("Counter_Overflow", |b| { b.iter(|| { - let mut rng = SmallRng::from_entropy(); - let _i1 = rng.gen_range(0..4); - let _i2 = rng.gen_range(0..4); - let _i3 = rng.gen_range(0..10); - let _i4 = rng.gen_range(0..10); - let _i5 = rng.gen_range(0..10); + // 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_forth_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_forth_attribute]), + ], + ); }); }); diff --git a/opentelemetry-sdk/src/metrics/internal/aggregate.rs b/opentelemetry-sdk/src/metrics/internal/aggregate.rs index 13e63b33c3..93fcc6a69c 100644 --- a/opentelemetry-sdk/src/metrics/internal/aggregate.rs +++ b/opentelemetry-sdk/src/metrics/internal/aggregate.rs @@ -24,7 +24,7 @@ pub(crate) static STREAM_OVERFLOW_ATTRIBUTE_SET: Lazy = Lazy::new( /// Checks whether aggregator has hit cardinality limit for metric streams pub(crate) fn is_under_cardinality_limit(size: usize) -> bool { - size < STREAM_CARDINALITY_LIMIT as usize - 1 + size < STREAM_CARDINALITY_LIMIT as usize } /// Receives measurements to be aggregated. diff --git a/opentelemetry-sdk/src/metrics/internal/sum.rs b/opentelemetry-sdk/src/metrics/internal/sum.rs index b46f4c952c..27710b0743 100644 --- a/opentelemetry-sdk/src/metrics/internal/sum.rs +++ b/opentelemetry-sdk/src/metrics/internal/sum.rs @@ -55,12 +55,12 @@ impl> ValueMap { Entry::Vacant(vacant_entry) => { if is_under_cardinality_limit(size) { vacant_entry.insert(measurement); + } else if let Some(val) = values.get_mut(&STREAM_OVERFLOW_ATTRIBUTE_SET) { + *val += measurement; + return; } else { - values - .entry(STREAM_OVERFLOW_ATTRIBUTE_SET.clone()) - .and_modify(|val| *val += measurement) - .or_insert(measurement); - global::handle_error(MetricsError::Other("Warning: Maximum data points for metric stream exceeded. Entry added to overflow.".into())); + values.insert(STREAM_OVERFLOW_ATTRIBUTE_SET.clone(), measurement); + global::handle_error(MetricsError::Other("Warning: Maximum data points for metric stream exceeded. Entry added to overflow. Subsequent overflows to same metric until next collect will not be logged.".into())); } } } diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index 830f7a7d35..1bad037ad4 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -161,6 +161,35 @@ 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)] + async fn counter_overflow_delta() { + // Arrange + let mut test_context = TestContext::new(Temporality::Delta); + let counter = test_context.u64_counter("test", "my_counter", None); + + // Act + // Record measurements with A:0, A:1,.......A:1999, which just fits in the 2000 limit + for v in 0..2000 { + counter.add(100, &[KeyValue::new("A", v.to_string())]); + } + + // All of the below will now go into overflow. + counter.add(100, &[KeyValue::new("A", "foo")]); + counter.add(100, &[KeyValue::new("A", "another")]); + counter.add(100, &[KeyValue::new("A", "yet_another")]); + test_context.flush_metrics(); + + let sum = test_context.get_aggregation::>("my_counter", None); + + // Expecting 2001 metric points. (2000 + 1 overflow) + assert_eq!(sum.data_points.len(), 2001); + + let data_point = + find_datapoint_with_key_value(&sum.data_points, "otel.metric.overflow", "true") + .expect("overflow point expected"); + assert_eq!(data_point.value, 300); + } + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn counter_aggregation_cumulative() { // Run this test with stdout enabled to see output. diff --git a/stress/Cargo.toml b/stress/Cargo.toml index c693eef984..b28a367350 100644 --- a/stress/Cargo.toml +++ b/stress/Cargo.toml @@ -9,6 +9,11 @@ name = "metrics" path = "src/metrics.rs" doc = false +[[bin]] # Bin to run the metrics overflow stress tests +name = "metrics_overflow" +path = "src/metrics_overflow.rs" +doc = false + [[bin]] # Bin to run the logs stress tests name = "logs" path = "src/logs.rs" diff --git a/stress/src/metrics_overflow.rs b/stress/src/metrics_overflow.rs new file mode 100644 index 0000000000..ab8b6ac28c --- /dev/null +++ b/stress/src/metrics_overflow.rs @@ -0,0 +1,43 @@ +/* + Stress test results: + OS: Ubuntu 22.04.3 LTS (5.15.146.1-microsoft-standard-WSL2) + Hardware: AMD EPYC 7763 64-Core Processor - 2.44 GHz, 16vCPUs, + RAM: 64.0 GB + 4.5M /sec +*/ + +use lazy_static::lazy_static; +use opentelemetry::{ + metrics::{Counter, MeterProvider as _}, + KeyValue, +}; +use opentelemetry_sdk::metrics::{ManualReader, SdkMeterProvider}; +use rand::{rngs::SmallRng, Rng, SeedableRng}; +use std::borrow::Cow; + +mod throughput; + +lazy_static! { + static ref PROVIDER: SdkMeterProvider = SdkMeterProvider::builder() + .with_reader(ManualReader::builder().build()) + .build(); + static ref ATTRIBUTE_VALUES: [&'static str; 10] = [ + "value1", "value2", "value3", "value4", "value5", "value6", "value7", "value8", "value9", + "value10" + ]; + static ref COUNTER: Counter = PROVIDER + .meter(<&str as Into>>::into("test")) + .u64_counter("hello") + .init(); +} + +fn main() { + for v in 0..2001 { + COUNTER.add(100, &[KeyValue::new("A", v.to_string())]); + } + throughput::test_throughput(test_counter); +} + +fn test_counter() { + COUNTER.add(1, &[KeyValue::new("A", "2001")]); +} From 74b0ad41eb08315e06807a0b356f1a8b2cbf80ce Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 08:48:02 -0700 Subject: [PATCH 04/13] Fix metrics overflow stress test (#1819) --- stress/src/metrics_overflow.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/stress/src/metrics_overflow.rs b/stress/src/metrics_overflow.rs index ab8b6ac28c..0e27c96b24 100644 --- a/stress/src/metrics_overflow.rs +++ b/stress/src/metrics_overflow.rs @@ -12,8 +12,11 @@ use opentelemetry::{ KeyValue, }; use opentelemetry_sdk::metrics::{ManualReader, SdkMeterProvider}; -use rand::{rngs::SmallRng, Rng, SeedableRng}; -use std::borrow::Cow; +use rand::{ + rngs::{self}, + Rng, SeedableRng, +}; +use std::{borrow::Cow, cell::RefCell}; mod throughput; @@ -21,23 +24,26 @@ lazy_static! { static ref PROVIDER: SdkMeterProvider = SdkMeterProvider::builder() .with_reader(ManualReader::builder().build()) .build(); - static ref ATTRIBUTE_VALUES: [&'static str; 10] = [ - "value1", "value2", "value3", "value4", "value5", "value6", "value7", "value8", "value9", - "value10" - ]; static ref COUNTER: Counter = PROVIDER .meter(<&str as Into>>::into("test")) .u64_counter("hello") .init(); } +thread_local! { + /// Store random number generator for each thread + static CURRENT_RNG: RefCell = RefCell::new(rngs::SmallRng::from_entropy()); +} + fn main() { - for v in 0..2001 { - COUNTER.add(100, &[KeyValue::new("A", v.to_string())]); - } throughput::test_throughput(test_counter); } fn test_counter() { - COUNTER.add(1, &[KeyValue::new("A", "2001")]); + // The main goal of this test is to ensure that OTel SDK is not growing its + // memory usage indefinitely even when user code misbehaves by producing + // unbounded metric points (unique time series). + // It also checks that SDK's internal logging is also done in a bounded way. + let rand = CURRENT_RNG.with(|rng| rng.borrow_mut().gen_range(0..100000000)); + COUNTER.add(1, &[KeyValue::new("A", rand)]); } From 98741c7cc7b4aabd0a2c4f28ec2c26afb1117cd9 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 09:35:29 -0700 Subject: [PATCH 05/13] [Metrics] - Remove unit abstraction (#1821) Co-authored-by: Zhongyang Wu --- .cspell.json | 4 + examples/metrics-advanced/src/main.rs | 7 +- examples/metrics-basic/src/main.rs | 9 +- .../examples/basic-otlp-http/src/main.rs | 4 +- .../examples/basic-otlp/src/main.rs | 4 +- opentelemetry-proto/src/transform/metrics.rs | 2 +- opentelemetry-sdk/src/metrics/data/mod.rs | 4 +- opentelemetry-sdk/src/metrics/instrument.rs | 24 +-- opentelemetry-sdk/src/metrics/meter.rs | 179 +++++++----------- opentelemetry-sdk/src/metrics/mod.rs | 49 ++--- opentelemetry-sdk/src/metrics/pipeline.rs | 4 +- opentelemetry-sdk/src/metrics/view.rs | 2 +- opentelemetry-stdout/src/metrics/transform.rs | 21 +- opentelemetry/CHANGELOG.md | 3 + opentelemetry/src/metrics/instruments/mod.rs | 14 +- opentelemetry/src/metrics/mod.rs | 58 ++---- 16 files changed, 150 insertions(+), 238 deletions(-) diff --git a/.cspell.json b/.cspell.json index 57a1c7bd34..a091c48706 100644 --- a/.cspell.json +++ b/.cspell.json @@ -43,6 +43,8 @@ "Lalit", "LIBCLANG", "msrv", + "mykey", + "myvalue", "Ochtman", "opentelemetry", "OTLP", @@ -53,8 +55,10 @@ "runtimes", "rustc", "shoppingcart", + "struct", "Tescher", "tracerprovider", + "updown", "Zhongyang", "zipkin" ], diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index 6731071555..b99fc8d4e4 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -1,5 +1,4 @@ use opentelemetry::global; -use opentelemetry::metrics::Unit; use opentelemetry::Key; use opentelemetry::KeyValue; use opentelemetry_sdk::metrics::{ @@ -15,7 +14,7 @@ fn init_meter_provider() -> opentelemetry_sdk::metrics::SdkMeterProvider { Some( Stream::new() .name("my_histogram_renamed") - .unit(Unit::new("milliseconds")), + .unit("milliseconds"), ) } else { None @@ -76,7 +75,7 @@ async fn main() -> Result<(), Box> { // using view. let histogram = meter .f64_histogram("my_histogram") - .with_unit(Unit::new("ms")) + .with_unit("ms") .with_description("My histogram example description") .init(); @@ -114,7 +113,7 @@ async fn main() -> Result<(), Box> { // use a custom set of boundaries, and min/max values will not be recorded. let histogram2 = meter .f64_histogram("my_second_histogram") - .with_unit(Unit::new("ms")) + .with_unit("ms") .with_description("My histogram example description") .init(); diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index e965ab6efa..996b16a008 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -1,5 +1,4 @@ use opentelemetry::global; -use opentelemetry::metrics::Unit; use opentelemetry::KeyValue; use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider}; use opentelemetry_sdk::{runtime, Resource}; @@ -47,7 +46,7 @@ async fn main() -> Result<(), Box> { let _observable_counter = meter .u64_observable_counter("my_observable_counter") .with_description("My observable counter example description") - .with_unit(Unit::new("myunit")) + .with_unit("myunit") .with_callback(|observer| { observer.observe( 100, @@ -75,7 +74,7 @@ async fn main() -> Result<(), Box> { let _observable_up_down_counter = meter .i64_observable_up_down_counter("my_observable_updown_counter") .with_description("My observable updown counter example description") - .with_unit(Unit::new("myunit")) + .with_unit("myunit") .with_callback(|observer| { observer.observe( 100, @@ -108,7 +107,7 @@ async fn main() -> Result<(), Box> { let gauge = meter .f64_gauge("my_gauge") .with_description("A gauge set to 1.0") - .with_unit(Unit::new("myunit")) + .with_unit("myunit") .init(); gauge.record( @@ -123,7 +122,7 @@ async fn main() -> Result<(), Box> { let _observable_gauge = meter .f64_observable_gauge("my_observable_gauge") .with_description("An observable gauge set to 1.0") - .with_unit(Unit::new("myunit")) + .with_unit("myunit") .with_callback(|observer| { observer.observe( 1.0, diff --git a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs index 7cc6490164..4bb3f77c8f 100644 --- a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs @@ -1,7 +1,7 @@ use once_cell::sync::Lazy; use opentelemetry::{ global, - metrics::{MetricsError, Unit}, + metrics::MetricsError, trace::{TraceContextExt, TraceError, Tracer, TracerProvider as _}, Key, KeyValue, }; @@ -125,7 +125,7 @@ async fn main() -> Result<(), Box> { let counter = meter .u64_counter("test_counter") .with_description("a simple counter for demo purposes.") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .init(); for _ in 0..10 { counter.add(1, &[KeyValue::new("test_key", "test_value")]); diff --git a/opentelemetry-otlp/examples/basic-otlp/src/main.rs b/opentelemetry-otlp/examples/basic-otlp/src/main.rs index 605916801d..50c95723fc 100644 --- a/opentelemetry-otlp/examples/basic-otlp/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp/src/main.rs @@ -1,7 +1,7 @@ use once_cell::sync::Lazy; use opentelemetry::global; use opentelemetry::logs::LogError; -use opentelemetry::metrics::{MetricsError, Unit}; +use opentelemetry::metrics::MetricsError; use opentelemetry::trace::{TraceError, TracerProvider}; use opentelemetry::{ trace::{TraceContextExt, Tracer}, @@ -127,7 +127,7 @@ async fn main() -> Result<(), Box> { let counter = meter .u64_counter("test_counter") .with_description("a simple counter for demo purposes.") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .init(); for _ in 0..10 { counter.add(1, &[KeyValue::new("test_key", "test_value")]); diff --git a/opentelemetry-proto/src/transform/metrics.rs b/opentelemetry-proto/src/transform/metrics.rs index 610348baaf..f49fec06db 100644 --- a/opentelemetry-proto/src/transform/metrics.rs +++ b/opentelemetry-proto/src/transform/metrics.rs @@ -148,7 +148,7 @@ pub mod tonic { TonicMetric { name: metric.name.to_string(), description: metric.description.to_string(), - unit: metric.unit.as_str().to_string(), + unit: metric.unit.to_string(), metadata: vec![], // internal and currently unused data: metric.data.as_any().try_into().ok(), } diff --git a/opentelemetry-sdk/src/metrics/data/mod.rs b/opentelemetry-sdk/src/metrics/data/mod.rs index fd4c85c35f..64a1879d26 100644 --- a/opentelemetry-sdk/src/metrics/data/mod.rs +++ b/opentelemetry-sdk/src/metrics/data/mod.rs @@ -2,7 +2,7 @@ use std::{any, borrow::Cow, fmt, time::SystemTime}; -use opentelemetry::{metrics::Unit, KeyValue}; +use opentelemetry::KeyValue; use crate::{instrumentation::Scope, Resource}; @@ -38,7 +38,7 @@ pub struct Metric { /// The description of the instrument, which can be used in documentation. pub description: Cow<'static, str>, /// The unit in which the instrument reports. - pub unit: Unit, + pub unit: Cow<'static, str>, /// The aggregated data from an instrument. pub data: Box, } diff --git a/opentelemetry-sdk/src/metrics/instrument.rs b/opentelemetry-sdk/src/metrics/instrument.rs index 5b8ef5c6e5..72097d504d 100644 --- a/opentelemetry-sdk/src/metrics/instrument.rs +++ b/opentelemetry-sdk/src/metrics/instrument.rs @@ -3,7 +3,7 @@ use std::{any::Any, borrow::Cow, collections::HashSet, hash::Hash, marker, sync: use opentelemetry::{ metrics::{ AsyncInstrument, MetricsError, Result, SyncCounter, SyncGauge, SyncHistogram, - SyncUpDownCounter, Unit, + SyncUpDownCounter, }, Key, KeyValue, }; @@ -69,7 +69,7 @@ pub struct Instrument { /// The functional group of the instrument. pub kind: Option, /// Unit is the unit of measurement recorded by the instrument. - pub unit: Unit, + pub unit: Cow<'static, str>, /// The instrumentation that created the instrument. pub scope: Scope, } @@ -93,8 +93,8 @@ impl Instrument { } /// Set the instrument unit. - pub fn unit(mut self, unit: Unit) -> Self { - self.unit = unit; + pub fn unit(mut self, unit: impl Into>) -> Self { + self.unit = unit.into(); self } @@ -109,7 +109,7 @@ impl Instrument { self.name == "" && self.description == "" && self.kind.is_none() - && self.unit.as_str() == "" + && self.unit == "" && self.scope == Scope::default() } @@ -134,7 +134,7 @@ impl Instrument { } pub(crate) fn matches_unit(&self, other: &Instrument) -> bool { - self.unit.as_str() == "" || self.unit == other.unit + self.unit.is_empty() || self.unit.as_ref() == other.unit.as_ref() } pub(crate) fn matches_scope(&self, other: &Instrument) -> bool { @@ -171,7 +171,7 @@ pub struct Stream { /// Describes the purpose of the data. pub description: Cow<'static, str>, /// the unit of measurement recorded. - pub unit: Unit, + pub unit: Cow<'static, str>, /// Aggregation the stream uses for an instrument. pub aggregation: Option, /// An allow-list of attribute keys that will be preserved for the stream. @@ -201,8 +201,8 @@ impl Stream { } /// Set the stream unit. - pub fn unit(mut self, unit: Unit) -> Self { - self.unit = unit; + pub fn unit(mut self, unit: impl Into>) -> Self { + self.unit = unit.into(); self } @@ -233,7 +233,7 @@ pub(crate) struct InstrumentId { /// Defines the functional group of the instrument. pub(crate) kind: InstrumentKind, /// The unit of measurement recorded. - pub(crate) unit: Unit, + pub(crate) unit: Cow<'static, str>, /// Number is the underlying data type of the instrument. pub(crate) number: Cow<'static, str>, } @@ -306,7 +306,7 @@ pub(crate) struct IdInner { /// The functional group of the instrument. kind: InstrumentKind, /// The unit of measurement recorded by the instrument. - pub(crate) unit: Unit, + pub(crate) unit: Cow<'static, str>, /// The instrumentation that created the instrument. scope: Scope, } @@ -337,7 +337,7 @@ impl Observable { kind: InstrumentKind, name: Cow<'static, str>, description: Cow<'static, str>, - unit: Unit, + unit: Cow<'static, str>, measures: Vec>>, ) -> Self { Self { diff --git a/opentelemetry-sdk/src/metrics/meter.rs b/opentelemetry-sdk/src/metrics/meter.rs index 542c6e1281..26cf8167dd 100644 --- a/opentelemetry-sdk/src/metrics/meter.rs +++ b/opentelemetry-sdk/src/metrics/meter.rs @@ -7,7 +7,7 @@ use opentelemetry::{ noop::{NoopAsyncInstrument, NoopRegistration}, AsyncInstrument, Callback, CallbackRegistration, Counter, Gauge, Histogram, InstrumentProvider, MetricsError, ObservableCounter, ObservableGauge, - ObservableUpDownCounter, Observer as ApiObserver, Result, Unit, UpDownCounter, + ObservableUpDownCounter, Observer as ApiObserver, Result, UpDownCounter, }, KeyValue, }; @@ -84,50 +84,40 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.u64_resolver); - p.lookup( - InstrumentKind::Counter, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Counter::new(Arc::new(i))) + p.lookup(InstrumentKind::Counter, name, description, unit) + .map(|i| Counter::new(Arc::new(i))) } fn f64_counter( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); - p.lookup( - InstrumentKind::Counter, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Counter::new(Arc::new(i))) + p.lookup(InstrumentKind::Counter, name, description, unit) + .map(|i| Counter::new(Arc::new(i))) } fn u64_observable_counter( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.u64_resolver); let ms = p.measures( InstrumentKind::ObservableCounter, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableCounter::new(Arc::new(NoopAsyncInstrument::new()))); @@ -155,16 +145,16 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); let ms = p.measures( InstrumentKind::ObservableCounter, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableCounter::new(Arc::new(NoopAsyncInstrument::new()))); @@ -191,50 +181,40 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.i64_resolver); - p.lookup( - InstrumentKind::UpDownCounter, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| UpDownCounter::new(Arc::new(i))) + p.lookup(InstrumentKind::UpDownCounter, name, description, unit) + .map(|i| UpDownCounter::new(Arc::new(i))) } fn f64_up_down_counter( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); - p.lookup( - InstrumentKind::UpDownCounter, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| UpDownCounter::new(Arc::new(i))) + p.lookup(InstrumentKind::UpDownCounter, name, description, unit) + .map(|i| UpDownCounter::new(Arc::new(i))) } fn i64_observable_up_down_counter( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.i64_resolver); let ms = p.measures( InstrumentKind::ObservableUpDownCounter, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableUpDownCounter::new(Arc::new( @@ -264,16 +244,16 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); let ms = p.measures( InstrumentKind::ObservableUpDownCounter, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableUpDownCounter::new(Arc::new( @@ -303,67 +283,52 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.u64_resolver); - p.lookup( - InstrumentKind::Gauge, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Gauge::new(Arc::new(i))) + p.lookup(InstrumentKind::Gauge, name, description, unit) + .map(|i| Gauge::new(Arc::new(i))) } fn f64_gauge( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); - p.lookup( - InstrumentKind::Gauge, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Gauge::new(Arc::new(i))) + p.lookup(InstrumentKind::Gauge, name, description, unit) + .map(|i| Gauge::new(Arc::new(i))) } fn i64_gauge( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.i64_resolver); - p.lookup( - InstrumentKind::Gauge, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Gauge::new(Arc::new(i))) + p.lookup(InstrumentKind::Gauge, name, description, unit) + .map(|i| Gauge::new(Arc::new(i))) } fn u64_observable_gauge( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.u64_resolver); let ms = p.measures( InstrumentKind::ObservableGauge, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableGauge::new(Arc::new(NoopAsyncInstrument::new()))); @@ -391,16 +356,16 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.i64_resolver); let ms = p.measures( InstrumentKind::ObservableGauge, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableGauge::new(Arc::new(NoopAsyncInstrument::new()))); @@ -428,16 +393,16 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, callbacks: Vec>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); let ms = p.measures( InstrumentKind::ObservableGauge, name.clone(), description.clone(), - unit.clone().unwrap_or_default(), + unit.clone(), )?; if ms.is_empty() { return Ok(ObservableGauge::new(Arc::new(NoopAsyncInstrument::new()))); @@ -465,34 +430,24 @@ impl InstrumentProvider for SdkMeter { &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.f64_resolver); - p.lookup( - InstrumentKind::Histogram, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Histogram::new(Arc::new(i))) + p.lookup(InstrumentKind::Histogram, name, description, unit) + .map(|i| Histogram::new(Arc::new(i))) } fn u64_histogram( &self, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, ) -> Result> { - validate_instrument_config(name.as_ref(), unit.as_ref(), self.validation_policy)?; + validate_instrument_config(name.as_ref(), &unit, self.validation_policy)?; let p = InstrumentResolver::new(self, &self.u64_resolver); - p.lookup( - InstrumentKind::Histogram, - name, - description, - unit.unwrap_or_default(), - ) - .map(|i| Histogram::new(Arc::new(i))) + p.lookup(InstrumentKind::Histogram, name, description, unit) + .map(|i| Histogram::new(Arc::new(i))) } fn register_callback( @@ -563,7 +518,7 @@ enum InstrumentValidationPolicy { fn validate_instrument_config( name: &str, - unit: Option<&Unit>, + unit: &Option>, policy: InstrumentValidationPolicy, ) -> Result<()> { match validate_instrument_name(name).and_then(|_| validate_instrument_unit(unit)) { @@ -605,14 +560,14 @@ fn validate_instrument_name(name: &str) -> Result<()> { Ok(()) } -fn validate_instrument_unit(unit: Option<&Unit>) -> Result<()> { +fn validate_instrument_unit(unit: &Option>) -> Result<()> { if let Some(unit) = unit { - if unit.as_str().len() > INSTRUMENT_UNIT_NAME_MAX_LENGTH { + if unit.len() > INSTRUMENT_UNIT_NAME_MAX_LENGTH { return Err(MetricsError::InvalidInstrumentConfiguration( INSTRUMENT_UNIT_LENGTH, )); } - if unit.as_str().contains(|c: char| !c.is_ascii()) { + if unit.contains(|c: char| !c.is_ascii()) { return Err(MetricsError::InvalidInstrumentConfiguration( INSTRUMENT_UNIT_INVALID_CHAR, )); @@ -731,7 +686,7 @@ where kind: InstrumentKind, name: Cow<'static, str>, description: Option>, - unit: Unit, + unit: Option>, ) -> Result> { let aggregators = self.measures(kind, name, description, unit)?; Ok(ResolvedMeasures { @@ -744,12 +699,12 @@ where kind: InstrumentKind, name: Cow<'static, str>, description: Option>, - unit: Unit, + unit: Option>, ) -> Result>>> { let inst = Instrument { name, description: description.unwrap_or_default(), - unit, + unit: unit.unwrap_or_default(), kind: Some(kind), scope: self.meter.scope.clone(), }; @@ -762,7 +717,7 @@ where mod tests { use std::sync::Arc; - use opentelemetry::metrics::{InstrumentProvider, MeterProvider, MetricsError, Unit}; + use opentelemetry::metrics::{InstrumentProvider, MeterProvider, MetricsError}; use super::{ InstrumentValidationPolicy, SdkMeter, INSTRUMENT_NAME_FIRST_ALPHABETIC, @@ -895,7 +850,7 @@ mod tests { )); } }; - let unit = Some(Unit::new(unit)); + let unit = Some(unit.into()); assert( meter .u64_counter("test".into(), None, unit.clone()) diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index 1bad037ad4..e7a51213b0 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -10,10 +10,7 @@ //! //! ``` //! use opentelemetry::global; -//! use opentelemetry::{ -//! metrics::Unit, -//! KeyValue, -//! }; +//! use opentelemetry::KeyValue; //! use opentelemetry_sdk::{metrics::SdkMeterProvider, Resource}; //! //! // Generate SDK configuration, resource, views, etc @@ -29,7 +26,7 @@ //! // Create instruments scoped to the meter //! let counter = meter //! .u64_counter("power_consumption") -//! .with_unit(Unit::new("kWh")) +//! .with_unit("kWh") //! .init(); //! //! // use instruments to record measurements @@ -149,10 +146,7 @@ mod tests { use crate::testing::metrics::InMemoryMetricsExporterBuilder; use crate::{runtime, testing::metrics::InMemoryMetricsExporter}; use opentelemetry::metrics::{Counter, UpDownCounter}; - use opentelemetry::{ - metrics::{MeterProvider as _, Unit}, - KeyValue, - }; + use opentelemetry::{metrics::MeterProvider as _, KeyValue}; use std::borrow::Cow; // Run all tests in this mod @@ -232,7 +226,7 @@ mod tests { let meter = meter_provider.meter("test"); let _counter = meter .u64_observable_counter("my_observable_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_callback(|observer| { observer.observe(100, &[KeyValue::new("key1", "value1")]); observer.observe(200, &[KeyValue::new("key1", "value2")]); @@ -248,7 +242,7 @@ mod tests { assert!(!resource_metrics.is_empty()); let metric = &resource_metrics[0].scope_metrics[0].metrics[0]; assert_eq!(metric.name, "my_observable_counter"); - assert_eq!(metric.unit.as_str(), "my_unit"); + assert_eq!(metric.unit, "my_unit"); let sum = metric .data .as_any() @@ -312,13 +306,13 @@ mod tests { let meter = meter_provider.meter("test"); let counter = meter .u64_counter("my_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_description("my_description") .init(); let counter_duplicated = meter .u64_counter("my_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_description("my_description") .init(); @@ -338,7 +332,7 @@ mod tests { ); let metric = &resource_metrics[0].scope_metrics[0].metrics[0]; assert_eq!(metric.name, "my_counter"); - assert_eq!(metric.unit.as_str(), "my_unit"); + assert_eq!(metric.unit, "my_unit"); let sum = metric .data .as_any() @@ -364,13 +358,13 @@ mod tests { let meter2 = meter_provider.meter("test.meter2"); let counter1 = meter1 .u64_counter("my_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_description("my_description") .init(); let counter2 = meter2 .u64_counter("my_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_description("my_description") .init(); @@ -403,7 +397,7 @@ mod tests { if let Some(scope1) = scope1 { let metric1 = &scope1.metrics[0]; assert_eq!(metric1.name, "my_counter"); - assert_eq!(metric1.unit.as_str(), "my_unit"); + assert_eq!(metric1.unit, "my_unit"); assert_eq!(metric1.description, "my_description"); let sum1 = metric1 .data @@ -423,7 +417,7 @@ mod tests { if let Some(scope2) = scope2 { let metric2 = &scope2.metrics[0]; assert_eq!(metric2.name, "my_counter"); - assert_eq!(metric2.unit.as_str(), "my_unit"); + assert_eq!(metric2.unit, "my_unit"); assert_eq!(metric2.description, "my_description"); let sum2 = metric2 .data @@ -465,13 +459,13 @@ mod tests { ); let counter1 = meter1 .u64_counter("my_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_description("my_description") .init(); let counter2 = meter2 .u64_counter("my_counter") - .with_unit(Unit::new("my_unit")) + .with_unit("my_unit") .with_description("my_description") .init(); @@ -506,7 +500,7 @@ mod tests { let metric = &resource_metrics[0].scope_metrics[0].metrics[0]; assert_eq!(metric.name, "my_counter"); - assert_eq!(metric.unit.as_str(), "my_unit"); + assert_eq!(metric.unit, "my_unit"); assert_eq!(metric.description, "my_description"); let sum = metric .data @@ -536,7 +530,7 @@ mod tests { record_min_max: false, }) .name("test_histogram_renamed") - .unit(Unit::new("test_unit_renamed")); + .unit("test_unit_renamed"); let view = new_view(criteria, stream_invalid_aggregation).expect("Expected to create a new view"); @@ -549,7 +543,7 @@ mod tests { let meter = meter_provider.meter("test"); let histogram = meter .f64_histogram("test_histogram") - .with_unit(Unit::new("test_unit")) + .with_unit("test_unit") .init(); histogram.record(1.5, &[KeyValue::new("key1", "value1")]); @@ -566,8 +560,7 @@ mod tests { "View rename should be ignored and original name retained." ); assert_eq!( - metric.unit.as_str(), - "test_unit", + metric.unit, "test_unit", "View rename of unit should be ignored and original unit retained." ); } @@ -1157,7 +1150,7 @@ mod tests { let meter = self.meter_provider.meter(meter_name); let mut counter_builder = meter.u64_counter(counter_name); if let Some(unit_name) = unit { - counter_builder = counter_builder.with_unit(Unit::new(unit_name)); + counter_builder = counter_builder.with_unit(unit_name); } counter_builder.init() } @@ -1171,7 +1164,7 @@ mod tests { let meter = self.meter_provider.meter(meter_name); let mut updown_counter_builder = meter.i64_up_down_counter(counter_name); if let Some(unit_name) = unit { - updown_counter_builder = updown_counter_builder.with_unit(Unit::new(unit_name)); + updown_counter_builder = updown_counter_builder.with_unit(unit_name); } updown_counter_builder.init() } @@ -1217,7 +1210,7 @@ mod tests { let metric = &resource_metric.scope_metrics[0].metrics[0]; assert_eq!(metric.name, counter_name); if let Some(expected_unit) = unit_name { - assert_eq!(metric.unit.as_str(), expected_unit); + assert_eq!(metric.unit, expected_unit); } metric diff --git a/opentelemetry-sdk/src/metrics/pipeline.rs b/opentelemetry-sdk/src/metrics/pipeline.rs index cbb942b17e..49415c15e4 100644 --- a/opentelemetry-sdk/src/metrics/pipeline.rs +++ b/opentelemetry-sdk/src/metrics/pipeline.rs @@ -7,7 +7,7 @@ use std::{ use opentelemetry::{ global, - metrics::{CallbackRegistration, MetricsError, Result, Unit}, + metrics::{CallbackRegistration, MetricsError, Result}, KeyValue, }; @@ -192,7 +192,7 @@ impl SdkProducer for Pipeline { struct InstrumentSync { name: Cow<'static, str>, description: Cow<'static, str>, - unit: Unit, + unit: Cow<'static, str>, comp_agg: Box, } diff --git a/opentelemetry-sdk/src/metrics/view.rs b/opentelemetry-sdk/src/metrics/view.rs index 139ec05018..8184613589 100644 --- a/opentelemetry-sdk/src/metrics/view.rs +++ b/opentelemetry-sdk/src/metrics/view.rs @@ -160,7 +160,7 @@ pub fn new_view(criteria: Instrument, mask: Stream) -> Result> { } else { i.description.clone() }, - unit: if !mask.unit.as_str().is_empty() { + unit: if !mask.unit.is_empty() { mask.unit.clone() } else { i.unit.clone() diff --git a/opentelemetry-stdout/src/metrics/transform.rs b/opentelemetry-stdout/src/metrics/transform.rs index 49aa662696..9df88ff7da 100644 --- a/opentelemetry-stdout/src/metrics/transform.rs +++ b/opentelemetry-stdout/src/metrics/transform.rs @@ -60,29 +60,14 @@ impl From for ScopeMetrics { } } -#[derive(Serialize, Debug, Clone)] -struct Unit(Cow<'static, str>); - -impl Unit { - fn is_empty(&self) -> bool { - self.0.is_empty() - } -} - -impl From for Unit { - fn from(unit: opentelemetry::metrics::Unit) -> Self { - Unit(unit.as_str().to_string().into()) - } -} - #[derive(Serialize, Debug, Clone)] #[serde(rename_all = "camelCase")] struct Metric { name: Cow<'static, str>, #[serde(skip_serializing_if = "str::is_empty")] description: Cow<'static, str>, - #[serde(skip_serializing_if = "Unit::is_empty")] - unit: Unit, + #[serde(skip_serializing_if = "str::is_empty")] + unit: Cow<'static, str>, #[serde(flatten)] data: Option, } @@ -92,7 +77,7 @@ impl From for Metric { Metric { name: value.name, description: value.description, - unit: value.unit.into(), + unit: value.unit, data: map_data(value.data.as_any()), } } diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md index 396e48c388..1b578d9a2e 100644 --- a/opentelemetry/CHANGELOG.md +++ b/opentelemetry/CHANGELOG.md @@ -7,6 +7,9 @@ - When "metrics" feature is enabled, `KeyValue` implements `PartialEq`, `Eq`, `PartialOrder`, `Order`, `Hash`. This is meant to be used for metrics aggregation purposes only. +- Removed `Unit` struct for specifying Instrument units. Unit is treated as an + opaque string. Migration: Replace `.with_unit(Unit::new("myunit"))` with + `.with_unit("myunit")`. ## v0.23.0 diff --git a/opentelemetry/src/metrics/instruments/mod.rs b/opentelemetry/src/metrics/instruments/mod.rs index 67158cf565..3ee530453a 100644 --- a/opentelemetry/src/metrics/instruments/mod.rs +++ b/opentelemetry/src/metrics/instruments/mod.rs @@ -1,4 +1,4 @@ -use crate::metrics::{Meter, MetricsError, Result, Unit}; +use crate::metrics::{Meter, MetricsError, Result}; use crate::KeyValue; use core::fmt; use std::any::Any; @@ -29,7 +29,7 @@ pub struct InstrumentBuilder<'a, T> { instrument_provider: &'a dyn InstrumentProvider, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, _marker: marker::PhantomData, } @@ -61,8 +61,8 @@ where /// Unit must be: /// - ASCII string /// - No longer than 63 characters - pub fn with_unit(mut self, unit: Unit) -> Self { - self.unit = Some(unit); + pub fn with_unit>>(mut self, unit: S) -> Self { + self.unit = Some(unit.into()); self } @@ -112,7 +112,7 @@ where meter: &'a Meter, name: Cow<'static, str>, description: Option>, - unit: Option, + unit: Option>, _inst: marker::PhantomData, callbacks: Vec>, } @@ -147,8 +147,8 @@ where /// Unit must be: /// - ASCII string /// - No longer than 63 characters - pub fn with_unit(mut self, unit: Unit) -> Self { - self.unit = Some(unit); + pub fn with_unit>>(mut self, unit: S) -> Self { + self.unit = Some(unit.into()); self } diff --git a/opentelemetry/src/metrics/mod.rs b/opentelemetry/src/metrics/mod.rs index bd24d6368d..1bc17e476e 100644 --- a/opentelemetry/src/metrics/mod.rs +++ b/opentelemetry/src/metrics/mod.rs @@ -57,32 +57,6 @@ impl From> for MetricsError { } } -/// Units denote underlying data units tracked by `Meter`s. -#[derive(Clone, Default, Debug, PartialEq, Eq, Hash)] -pub struct Unit(Cow<'static, str>); - -impl Unit { - /// Create a new `Unit` from an `Into` - pub fn new(value: S) -> Self - where - S: Into>, - { - Unit(value.into()) - } - - /// View unit as &str - pub fn as_str(&self) -> &str { - self.0.as_ref() - } -} - -impl AsRef for Unit { - #[inline] - fn as_ref(&self) -> &str { - self.0.as_ref() - } -} - struct F64Hashable(f64); impl PartialEq for F64Hashable { @@ -139,7 +113,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Counter::new(Arc::new(noop::NoopSyncInstrument::new()))) } @@ -149,7 +123,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Counter::new(Arc::new(noop::NoopSyncInstrument::new()))) } @@ -159,7 +133,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableCounter::new(Arc::new( @@ -172,7 +146,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableCounter::new(Arc::new( @@ -185,7 +159,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(UpDownCounter::new( Arc::new(noop::NoopSyncInstrument::new()), @@ -197,7 +171,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(UpDownCounter::new( Arc::new(noop::NoopSyncInstrument::new()), @@ -209,7 +183,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableUpDownCounter::new(Arc::new( @@ -222,7 +196,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableUpDownCounter::new(Arc::new( @@ -235,7 +209,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Gauge::new(Arc::new(noop::NoopSyncInstrument::new()))) } @@ -245,7 +219,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Gauge::new(Arc::new(noop::NoopSyncInstrument::new()))) } @@ -255,7 +229,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Gauge::new(Arc::new(noop::NoopSyncInstrument::new()))) } @@ -265,7 +239,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableGauge::new(Arc::new( @@ -278,7 +252,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableGauge::new(Arc::new( @@ -291,7 +265,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, _callback: Vec>, ) -> Result> { Ok(ObservableGauge::new(Arc::new( @@ -304,7 +278,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Histogram::new(Arc::new(noop::NoopSyncInstrument::new()))) } @@ -314,7 +288,7 @@ pub trait InstrumentProvider { &self, _name: Cow<'static, str>, _description: Option>, - _unit: Option, + _unit: Option>, ) -> Result> { Ok(Histogram::new(Arc::new(noop::NoopSyncInstrument::new()))) } From 1b410d08dd9cd048fc349b7d8837fd7d114f7714 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 09:45:17 -0700 Subject: [PATCH 06/13] Remove AWSid generator (#1820) Co-authored-by: Zhongyang Wu --- opentelemetry-sdk/CHANGELOG.md | 15 +-- .../src/trace/id_generator/aws.rs | 98 ------------------- .../src/trace/id_generator/mod.rs | 3 - opentelemetry-sdk/src/trace/mod.rs | 5 - 4 files changed, 9 insertions(+), 112 deletions(-) delete mode 100644 opentelemetry-sdk/src/trace/id_generator/aws.rs diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 17e0483a5d..8460c89133 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -10,18 +10,21 @@ must now use `.with_resource(RESOURCE::default())` to configure Resource on logger provider. - Removed dependency on `ordered-float`. +- Removed `XrayIdGenerator`, which was marked deprecated since 0.21.3. Use + [`opentelemetry-aws`](https://crates.io/crates/opentelemetry-aws), version + 0.10.0 or newer. - **Breaking** [1726](https://github.com/open-telemetry/opentelemetry-rust/pull/1726) Update `LogProcessor::emit() method to take mutable reference to LogData. This is breaking - change for LogProcessor developers. If the processor needs to invoke the exporter - asynchronously, it should clone the data to ensure it can be safely processed without + change for LogProcessor developers. If the processor needs to invoke the exporter + asynchronously, it should clone the data to ensure it can be safely processed without lifetime issues. Any changes made to the log data before cloning in this method will be reflected in the next log processor in the chain, as well as to the exporter. - **Breaking** [1726](https://github.com/open-telemetry/opentelemetry-rust/pull/1726) - Update `LogExporter::export() method to accept a batch of log data, which can be either a - reference or owned `LogData`. If the exporter needs to process the log data - asynchronously, it should clone the log data to ensure it can be safely processed without - lifetime issues. + Update `LogExporter::export() method to accept a batch of log data, which can be either a + reference or owned`LogData`. If the exporter needs to process the log data + asynchronously, it should clone the log data to ensure it can be safely processed without + lifetime issues. ## v0.23.0 diff --git a/opentelemetry-sdk/src/trace/id_generator/aws.rs b/opentelemetry-sdk/src/trace/id_generator/aws.rs deleted file mode 100644 index 278467e230..0000000000 --- a/opentelemetry-sdk/src/trace/id_generator/aws.rs +++ /dev/null @@ -1,98 +0,0 @@ -use crate::trace::{IdGenerator, RandomIdGenerator}; -use opentelemetry::trace::{SpanId, TraceId}; -use std::time::{Duration, UNIX_EPOCH}; - -/// Generates AWS X-Ray compliant Trace and Span ids. -/// -/// Generates OpenTelemetry formatted `TraceId`'s and `SpanId`'s. The `TraceId`'s are generated so -/// they can be backed out into X-Ray format by the [AWS X-Ray Exporter][xray-exporter] in the -/// [OpenTelemetry Collector][otel-collector]. -/// -/// ## Trace ID Format -/// -/// A `trace_id` consists of three numbers separated by hyphens. For example, `1-58406520-a006649127e371903a2de979`. -/// This includes: -/// -/// * The version number, that is, 1. -/// * The time of the original request, in Unix epoch time, in 8 hexadecimal digits. -/// * For example, 10:00AM December 1st, 2016 PST in epoch time is 1480615200 seconds, or 58406520 in hexadecimal digits. -/// * A 96-bit identifier for the trace, globally unique, in 24 hexadecimal digits. -/// -/// See the [AWS X-Ray Documentation][xray-trace-id] for more details. -/// -/// ## Example -/// -/// ``` -/// use opentelemetry_sdk::trace::{self, TracerProvider, XrayIdGenerator}; -/// -/// let _provider: TracerProvider = TracerProvider::builder() -/// .with_config(trace::Config::default().with_id_generator(XrayIdGenerator::default())) -/// .build(); -/// ``` -/// -/// [otel-collector]: https://github.com/open-telemetry/opentelemetry-collector-contrib#opentelemetry-collector-contrib -/// [xray-exporter]: https://godoc.org/github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsxrayexporter -/// [xray-trace-id]: https://docs.aws.amazon.com/xray/latest/devguide/xray-api-sendingdata.html#xray-api-traceids -#[derive(Debug, Default)] -#[deprecated( - since = "0.21.3", - note = "XrayId Generator has been migrated to the opentelemetry-aws crate" -)] -pub struct XrayIdGenerator { - sdk_default_generator: RandomIdGenerator, -} - -impl IdGenerator for XrayIdGenerator { - /// Generates a new `TraceId` that can be converted to an X-Ray Trace ID - fn new_trace_id(&self) -> TraceId { - let mut default_trace_id: String = - format!("{:024x}", self.sdk_default_generator.new_trace_id()); - - default_trace_id.truncate(24); - - let epoch_time_seconds: u64 = opentelemetry::time::now() - .duration_since(UNIX_EPOCH) - .unwrap_or_else(|_| Duration::from_secs(0)) - .as_secs(); - - TraceId::from_hex(format!("{:08x}{}", epoch_time_seconds, default_trace_id).as_str()) - .unwrap_or(TraceId::INVALID) - } - - /// Generates a new `SpanId` that can be converted to an X-Ray Segment ID - fn new_span_id(&self) -> SpanId { - self.sdk_default_generator.new_span_id() - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::thread::sleep; - - #[test] - fn test_trace_id_generation() { - let before: u64 = opentelemetry::time::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_secs(); - sleep(Duration::from_secs(1)); - - let generator: XrayIdGenerator = XrayIdGenerator::default(); - let trace_id: TraceId = generator.new_trace_id(); - - sleep(Duration::from_secs(1)); - let after: u64 = opentelemetry::time::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_secs(); - - let trace_as_hex = trace_id.to_string(); - let (timestamp, _xray_id) = trace_as_hex.split_at(8_usize); - - let trace_time: u64 = u64::from_str_radix(timestamp, 16).unwrap(); - - assert!(before <= trace_time); - assert!(after >= trace_time); - } -} diff --git a/opentelemetry-sdk/src/trace/id_generator/mod.rs b/opentelemetry-sdk/src/trace/id_generator/mod.rs index 27cea4cb19..1cf483118e 100644 --- a/opentelemetry-sdk/src/trace/id_generator/mod.rs +++ b/opentelemetry-sdk/src/trace/id_generator/mod.rs @@ -1,6 +1,3 @@ -//! Id Generator -pub(super) mod aws; - use opentelemetry::trace::{SpanId, TraceId}; use rand::{rngs, Rng, SeedableRng}; use std::cell::RefCell; diff --git a/opentelemetry-sdk/src/trace/mod.rs b/opentelemetry-sdk/src/trace/mod.rs index 36c017dc7d..d6c4ceae2e 100644 --- a/opentelemetry-sdk/src/trace/mod.rs +++ b/opentelemetry-sdk/src/trace/mod.rs @@ -20,11 +20,6 @@ mod tracer; pub use config::{config, Config}; pub use events::SpanEvents; -#[deprecated( - since = "0.21.3", - note = "XrayId Generator has been migrated to the opentelemetry-aws crate" -)] -pub use id_generator::aws::XrayIdGenerator; pub use id_generator::{IdGenerator, RandomIdGenerator}; pub use links::SpanLinks; pub use provider::{Builder, TracerProvider}; From f8ee5518d75ea67e0a3be3a7b9c0b45274cbf8ed Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 10:10:20 -0700 Subject: [PATCH 07/13] Improve CI speed by removing duplicate run of tests (#1822) --- scripts/test.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/test.sh b/scripts/test.sh index cd02a14994..dfcb925659 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -2,13 +2,15 @@ set -eu -cargo test --workspace --all-features "$@" -- --test-threads=1 +echo "Running tests for all packages in workspace with --all-features" +cargo test --workspace --all-features # See https://github.com/rust-lang/cargo/issues/5364 +echo "Running tests for opentelemetry package with --no-default-features" cargo test --manifest-path=opentelemetry/Cargo.toml --no-default-features # Run global tracer provider test in single thread -cargo test --manifest-path=opentelemetry/Cargo.toml --all-features -- --ignored --test-threads=1 - -cargo test --manifest-path=opentelemetry/Cargo.toml --all-features -cargo test --manifest-path=opentelemetry-zipkin/Cargo.toml --all-features +# //TODO: This tests were not running for a while. Need to find out how to run +# run them. Using --ignored will run other tests as well, so that cannot be used. +# echo "Running global tracer provider for opentelemetry-sdk package with single thread." +# cargo test --manifest-path=opentelemetry-sdk/Cargo.toml --all-features -- --test-threads=1 From 57f87d68ffd098877e327aada5b9ad7499aa0b9b Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 10:55:24 -0700 Subject: [PATCH 08/13] Add more observable counter tests (#1818) --- .cspell.json | 3 + opentelemetry-sdk/src/metrics/mod.rs | 152 +++++++++++++++------------ 2 files changed, 85 insertions(+), 70 deletions(-) diff --git a/.cspell.json b/.cspell.json index a091c48706..47fc5d18e8 100644 --- a/.cspell.json +++ b/.cspell.json @@ -32,8 +32,10 @@ "Cijo", "clippy", "codecov", + "datapoint", "deque", "Dirkjan", + "EPYC", "hasher", "isahc", "Isobel", @@ -45,6 +47,7 @@ "msrv", "mykey", "myvalue", + "nocapture", "Ochtman", "opentelemetry", "OTLP", diff --git a/opentelemetry-sdk/src/metrics/mod.rs b/opentelemetry-sdk/src/metrics/mod.rs index e7a51213b0..ca5076763d 100644 --- a/opentelemetry-sdk/src/metrics/mod.rs +++ b/opentelemetry-sdk/src/metrics/mod.rs @@ -145,9 +145,10 @@ mod tests { use crate::metrics::reader::TemporalitySelector; use crate::testing::metrics::InMemoryMetricsExporterBuilder; use crate::{runtime, testing::metrics::InMemoryMetricsExporter}; - use opentelemetry::metrics::{Counter, UpDownCounter}; + use opentelemetry::metrics::{Counter, Meter, UpDownCounter}; use opentelemetry::{metrics::MeterProvider as _, KeyValue}; use std::borrow::Cow; + use std::sync::{Arc, Mutex}; // Run all tests in this mod // cargo test metrics::tests --features=metrics,testing @@ -213,86 +214,94 @@ mod tests { } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] - async fn observable_counter_aggregation() { + async fn observable_counter_aggregation_cumulative_non_zero_increment() { // Run this test with stdout enabled to see output. - // cargo test observable_counter_aggregation --features=metrics,testing -- --nocapture + // cargo test observable_counter_aggregation_cumulative_non_zero_increment --features=testing -- --nocapture + observable_counter_aggregation_helper(Temporality::Cumulative, 100, 10, 4); + } - // Arrange - let exporter = InMemoryMetricsExporter::default(); - let reader = PeriodicReader::builder(exporter.clone(), runtime::Tokio).build(); - let meter_provider = SdkMeterProvider::builder().with_reader(reader).build(); + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn observable_counter_aggregation_delta_non_zero_increment() { + // Run this test with stdout enabled to see output. + // cargo test observable_counter_aggregation_delta_non_zero_increment --features=testing -- --nocapture + observable_counter_aggregation_helper(Temporality::Delta, 100, 10, 4); + } - // Act - let meter = meter_provider.meter("test"); - let _counter = meter + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + async fn observable_counter_aggregation_cumulative_zero_increment() { + // Run this test with stdout enabled to see output. + // cargo test observable_counter_aggregation_cumulative_zero_increment --features=testing -- --nocapture + observable_counter_aggregation_helper(Temporality::Cumulative, 100, 0, 4); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] + #[ignore = "Aggregation bug! https://github.com/open-telemetry/opentelemetry-rust/issues/1517"] + async fn observable_counter_aggregation_delta_zero_increment() { + // Run this test with stdout enabled to see output. + // cargo test observable_counter_aggregation_delta_zero_increment --features=testing -- --nocapture + observable_counter_aggregation_helper(Temporality::Delta, 100, 0, 4); + } + + fn observable_counter_aggregation_helper( + temporality: Temporality, + start: u64, + increment: u64, + length: u64, + ) { + // Arrange + let mut test_context = TestContext::new(temporality); + // The Observable counter reports values[0], values[1],....values[n] on each flush. + let values: Vec = (0..length).map(|i| start + i * increment).collect(); + println!("Testing with observable values: {:?}", values); + let values = Arc::new(values); + let values_clone = values.clone(); + let i = Arc::new(Mutex::new(0)); + let _observable_counter = test_context + .meter() .u64_observable_counter("my_observable_counter") .with_unit("my_unit") - .with_callback(|observer| { - observer.observe(100, &[KeyValue::new("key1", "value1")]); - observer.observe(200, &[KeyValue::new("key1", "value2")]); + .with_callback(move |observer| { + let mut index = i.lock().unwrap(); + if *index < values.len() { + observer.observe(values[*index], &[KeyValue::new("key1", "value1")]); + *index += 1; + } }) .init(); - 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_observable_counter"); - assert_eq!(metric.unit, "my_unit"); - let sum = metric - .data - .as_any() - .downcast_ref::>() - .expect("Sum aggregation expected for ObservableCounter 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::Cumulative, - "Should produce cumulative by default." - ); - - // find and validate key1=value1 datapoint - let mut data_point1 = None; - for datapoint in &sum.data_points { - if datapoint - .attributes - .iter() - .any(|kv| kv.key.as_str() == "key1" && kv.value.as_str() == "value1") - { - data_point1 = Some(datapoint); + for (iter, v) in values_clone.iter().enumerate() { + test_context.flush_metrics(); + let sum = test_context.get_aggregation::>("my_observable_counter", None); + assert_eq!(sum.data_points.len(), 1); + assert!(sum.is_monotonic, "Counter should produce monotonic."); + if let Temporality::Cumulative = temporality { + assert_eq!( + sum.temporality, + Temporality::Cumulative, + "Should produce cumulative" + ); + } else { + assert_eq!(sum.temporality, Temporality::Delta, "Should produce delta"); } - } - assert_eq!( - data_point1 - .expect("datapoint with key1=value1 expected") - .value, - 100 - ); - // find and validate key1=value2 datapoint - let mut data_point1 = None; - for datapoint in &sum.data_points { - if datapoint - .attributes - .iter() - .any(|kv| kv.key.as_str() == "key1" && kv.value.as_str() == "value2") - { - data_point1 = Some(datapoint); + // find and validate key1=value1 datapoint + let data_point = find_datapoint_with_key_value(&sum.data_points, "key1", "value1") + .expect("datapoint with key1=value1 expected"); + if let Temporality::Cumulative = temporality { + // Cumulative counter should have the value as is. + assert_eq!(data_point.value, *v); + } else { + // Delta counter should have the increment value. + // Except for the first value which should be the start value. + if iter == 0 { + assert_eq!(data_point.value, start); + } else { + assert_eq!(data_point.value, increment); + } } + + test_context.reset_metrics(); } - assert_eq!( - data_point1 - .expect("datapoint with key1=value2 expected") - .value, - 200 - ); } #[tokio::test(flavor = "multi_thread", worker_threads = 1)] @@ -566,7 +575,6 @@ mod tests { } #[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() { // cargo test spatial_aggregation_when_view_drops_attributes_observable_counter --features=metrics,testing @@ -1169,6 +1177,10 @@ mod tests { updown_counter_builder.init() } + fn meter(&self) -> Meter { + self.meter_provider.meter("test") + } + fn flush_metrics(&self) { self.meter_provider.force_flush().unwrap(); } From ac741c2186014498168315664e090118df616d5a Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 11:45:49 -0700 Subject: [PATCH 09/13] Remove env_logger from cargo (#1825) --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 0c8e3839b4..5f16c4957d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,6 @@ debug = 1 async-std = "1.10" async-trait = "0.1" bytes = "1" -env_logger = { version = "0.10", default-features = false } # env_logger requires a newer MSRV futures-core = "0.3" futures-executor = "0.3" futures-util = { version = "0.3", default-features = false } From ec9fd62d2ef6ebdcc736d26258758c11c8650b5f Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 12:52:35 -0700 Subject: [PATCH 10/13] Run msrv check for all crates (#1826) --- .github/workflows/ci.yml | 6 ++---- scripts/msrv.sh | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) create mode 100755 scripts/msrv.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 295c69bc6b..18a18f849d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,10 +110,8 @@ jobs: - uses: dtolnay/rust-toolchain@1.65.0 - name: Patch dependencies versions # some dependencies bump MSRV without major version bump run: bash ./scripts/patch_dependencies.sh - - name: Run tests - run: cargo --version && - cargo test --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,testing && - cargo test --manifest-path=opentelemetry-zipkin/Cargo.toml + - name: Check MSRV for all crates + run: bash ./scripts/msrv.sh cargo-deny: runs-on: ubuntu-latest # This uses the step `EmbarkStudios/cargo-deny-action@v1` which is only supported on Linux continue-on-error: true # Prevent sudden announcement of a new advisory from failing ci diff --git a/scripts/msrv.sh b/scripts/msrv.sh new file mode 100755 index 0000000000..eb9386cfa3 --- /dev/null +++ b/scripts/msrv.sh @@ -0,0 +1,33 @@ +#!/bin/bash + +set -eu + +echo "Running msrv check for opentelemetry package" +cargo check --manifest-path=opentelemetry/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-sdk package" +cargo check --manifest-path=opentelemetry-sdk/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-stdout package" +cargo check --manifest-path=opentelemetry-stdout/Cargo.toml --all-features + +# TODO: Ignoring as this is failing with the following error: +# error: package `prost-derive v0.12.6` cannot be built because it requires rustc 1.70 or newer, while the currently active rustc version is 1.65.0 +#echo "Running msrv check for opentelemetry-otlp package" +# cargo check --manifest-path=opentelemetry-otlp/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-http package" +cargo check --manifest-path=opentelemetry-http/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-jaeger-propagator package" +cargo check --manifest-path=opentelemetry-jaeger-propagator/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-zipkin package" +cargo check --manifest-path=opentelemetry-zipkin/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-appender-log package" +cargo check --manifest-path=opentelemetry-appender-log/Cargo.toml --all-features + +echo "Running msrv check for opentelemetry-appender-tracing package" +cargo check --manifest-path=opentelemetry-appender-tracing/Cargo.toml --all-features + From 28f8dbca39896e94cb7c238bc3504aa71ec10f0f Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Fri, 24 May 2024 13:01:56 -0700 Subject: [PATCH 11/13] Add note about using newer protoc in contributing doc (#1824) --- CONTRIBUTING.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4d0d9e9e9b..fe44403566 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,6 +37,11 @@ You can provide the protocol compiler protoc path programmatically (only works w export PROTOC=$(which protoc) ``` +It is recommended to use "3.15" or newer of protoc, as some of the proto +definitions include "optional" fields, that are not supported in older versions, +resulting in errors as shown +[here](https://github.com/open-telemetry/opentelemetry-proto/issues/451). + Prerequisites to build the protocol compiler protoc from source - [protoc](https://github.com/protocolbuffers/protobuf) From b1a80eed9fc65e339e6c636f4755515bcc2b098d Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Fri, 24 May 2024 13:22:29 -0700 Subject: [PATCH 12/13] OtlpTracePipeline fixed to no longer set global provider (#1812) Co-authored-by: Zhongyang Wu Co-authored-by: Cijo Thomas --- examples/tracing-jaeger/src/main.rs | 5 ++- opentelemetry-otlp/CHANGELOG.md | 2 + .../examples/basic-otlp-http/src/main.rs | 7 +++- .../examples/basic-otlp/src/main.rs | 6 ++- opentelemetry-otlp/src/lib.rs | 6 ++- opentelemetry-otlp/src/logs.rs | 8 ++-- opentelemetry-otlp/src/span.rs | 38 ++++++------------- .../tests/integration_test/tests/traces.rs | 8 ++-- opentelemetry-otlp/tests/smoke.rs | 9 ++++- 9 files changed, 44 insertions(+), 45 deletions(-) diff --git a/examples/tracing-jaeger/src/main.rs b/examples/tracing-jaeger/src/main.rs index 9a76fca48f..5ddb5a843c 100644 --- a/examples/tracing-jaeger/src/main.rs +++ b/examples/tracing-jaeger/src/main.rs @@ -10,7 +10,7 @@ use opentelemetry_semantic_conventions::resource::SERVICE_NAME; use std::error::Error; -fn init_tracer() -> Result { +fn init_tracer_provider() -> Result { opentelemetry_otlp::new_pipeline() .tracing() .with_exporter( @@ -29,7 +29,8 @@ fn init_tracer() -> Result { #[tokio::main] async fn main() -> Result<(), Box> { - let _tracer = init_tracer().expect("Failed to initialize tracer."); + let tracer_provider = init_tracer_provider().expect("Failed to initialize tracer provider."); + global::set_tracer_provider(tracer_provider.clone()); let tracer = global::tracer("tracing-jaeger"); tracer.in_span("main-operation", |cx| { diff --git a/opentelemetry-otlp/CHANGELOG.md b/opentelemetry-otlp/CHANGELOG.md index 07213b96aa..a2a598f4ac 100644 --- a/opentelemetry-otlp/CHANGELOG.md +++ b/opentelemetry-otlp/CHANGELOG.md @@ -12,6 +12,8 @@ Instead of using `.with_config(Config::default().with_resource(RESOURCE::default()))` users must now use `.with_resource(RESOURCE::default())` to configure Resource when using `OtlpLogPipeline`. +- **Breaking** The methods `OtlpTracePipeline::install_simple()` and `OtlpTracePipeline::install_batch()` would now return `TracerProvider` instead of `Tracer`. + These methods would also no longer set the global tracer provider. It would now be the responsibility of users to set it by calling `global::set_tracer_provider(tracer_provider.clone());`. Refer to the [basic-otlp](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/examples/basic-otlp/src/main.rs) and [basic-otlp-http](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs) examples on how to initialize OTLP Trace Exporter. ## v0.16.0 diff --git a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs index 4bb3f77c8f..c237c484e0 100644 --- a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs @@ -37,7 +37,7 @@ fn init_logs() -> Result .install_batch(opentelemetry_sdk::runtime::Tokio) } -fn init_tracer() -> Result { +fn init_tracer_provider() -> Result { opentelemetry_otlp::new_pipeline() .tracing() .with_exporter( @@ -67,13 +67,16 @@ fn init_metrics() -> Result Result<(), Box> { - let result = init_tracer(); + let result = init_tracer_provider(); assert!( result.is_ok(), "Init tracer failed with error: {:?}", result.err() ); + let tracer_provider = result.unwrap(); + global::set_tracer_provider(tracer_provider.clone()); + let result = init_metrics(); assert!( result.is_ok(), diff --git a/opentelemetry-otlp/examples/basic-otlp/src/main.rs b/opentelemetry-otlp/examples/basic-otlp/src/main.rs index 50c95723fc..2b12c11212 100644 --- a/opentelemetry-otlp/examples/basic-otlp/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp/src/main.rs @@ -22,7 +22,7 @@ static RESOURCE: Lazy = Lazy::new(|| { )]) }); -fn init_tracer() -> Result { +fn init_tracer_provider() -> Result { opentelemetry_otlp::new_pipeline() .tracing() .with_exporter( @@ -72,12 +72,14 @@ async fn main() -> Result<(), Box> { // matches the containing block, reporting traces and metrics during the whole // execution. - let result = init_tracer(); + let result = init_tracer_provider(); assert!( result.is_ok(), "Init tracer failed with error: {:?}", result.err() ); + let tracer_provider = result.unwrap(); + global::set_tracer_provider(tracer_provider.clone()); let result = init_metrics(); assert!( diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 76e3493dab..a3d9b02e2d 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -120,7 +120,7 @@ //! on the choice of exporters. //! //! ```no_run -//! use opentelemetry::{KeyValue, trace::Tracer}; +//! use opentelemetry::{global, KeyValue, trace::Tracer}; //! use opentelemetry_sdk::{trace::{self, RandomIdGenerator, Sampler}, Resource}; //! # #[cfg(feature = "metrics")] //! use opentelemetry_sdk::metrics::reader::{DefaultAggregationSelector, DefaultTemporalitySelector}; @@ -138,7 +138,7 @@ //! map.insert("x-number", "123".parse().unwrap()); //! map.insert_bin("trace-proto-bin", MetadataValue::from_bytes(b"[binary data]")); //! -//! let tracer = opentelemetry_otlp::new_pipeline() +//! let tracer_provider = opentelemetry_otlp::new_pipeline() //! .tracing() //! .with_exporter( //! opentelemetry_otlp::new_exporter() @@ -157,6 +157,8 @@ //! .with_resource(Resource::new(vec![KeyValue::new("service.name", "example")])), //! ) //! .install_batch(opentelemetry_sdk::runtime::Tokio)?; +//! global::set_tracer_provider(tracer_provider); +//! let tracer = global::tracer("tracer-name"); //! # tracer //! # }; //! diff --git a/opentelemetry-otlp/src/logs.rs b/opentelemetry-otlp/src/logs.rs index 72e3b352ec..3f21697fb0 100644 --- a/opentelemetry-otlp/src/logs.rs +++ b/opentelemetry-otlp/src/logs.rs @@ -151,9 +151,9 @@ impl OtlpLogPipeline { impl OtlpLogPipeline { /// Install the configured log exporter. /// - /// Returns a [`Logger`] with the name `opentelemetry-otlp` and the current crate version. + /// Returns a [`LoggerProvider`]. /// - /// [`Logger`]: opentelemetry_sdk::logs::Logger + /// [`LoggerProvider`]: opentelemetry_sdk::logs::LoggerProvider pub fn install_simple(self) -> Result { Ok(build_simple_with_exporter( self.exporter_builder.build_log_exporter()?, @@ -164,9 +164,9 @@ impl OtlpLogPipeline { /// Install the configured log exporter and a batch log processor using the /// specified runtime. /// - /// Returns a [`Logger`] with the name `opentelemetry-otlp` and the current crate version. + /// Returns a [`LoggerProvider`]. /// - /// [`Logger`]: opentelemetry_sdk::logs::Logger + /// [`LoggerProvider`]: opentelemetry_sdk::logs::LoggerProvider pub fn install_batch( self, runtime: R, diff --git a/opentelemetry-otlp/src/span.rs b/opentelemetry-otlp/src/span.rs index 4b658a89e2..ba70f0825e 100644 --- a/opentelemetry-otlp/src/span.rs +++ b/opentelemetry-otlp/src/span.rs @@ -5,10 +5,7 @@ use std::fmt::Debug; use futures_core::future::BoxFuture; -use opentelemetry::{ - global, - trace::{TraceError, TracerProvider}, -}; +use opentelemetry::trace::TraceError; use opentelemetry_sdk::{ self as sdk, export::trace::{ExportResult, SpanData}, @@ -99,10 +96,10 @@ impl OtlpTracePipeline { impl OtlpTracePipeline { /// Install the configured span exporter. /// - /// Returns a [`Tracer`] with the name `opentelemetry-otlp` and current crate version. + /// Returns a [`TracerProvider`]. /// - /// [`Tracer`]: opentelemetry::trace::Tracer - pub fn install_simple(self) -> Result { + /// [`TracerProvider`]: opentelemetry::trace::TracerProvider + pub fn install_simple(self) -> Result { Ok(build_simple_with_exporter( self.exporter_builder.build_span_exporter()?, self.trace_config, @@ -112,15 +109,15 @@ impl OtlpTracePipeline { /// Install the configured span exporter and a batch span processor using the /// specified runtime. /// - /// Returns a [`Tracer`] with the name `opentelemetry-otlp` and current crate version. + /// Returns a [`TracerProvider`]. /// /// `install_batch` will panic if not called within a tokio runtime /// - /// [`Tracer`]: opentelemetry::trace::Tracer + /// [`TracerProvider`]: opentelemetry::trace::TracerProvider pub fn install_batch( self, runtime: R, - ) -> Result { + ) -> Result { Ok(build_batch_with_exporter( self.exporter_builder.build_span_exporter()?, self.trace_config, @@ -133,18 +130,13 @@ impl OtlpTracePipeline { fn build_simple_with_exporter( exporter: SpanExporter, trace_config: Option, -) -> sdk::trace::Tracer { +) -> sdk::trace::TracerProvider { let mut provider_builder = sdk::trace::TracerProvider::builder().with_simple_exporter(exporter); if let Some(config) = trace_config { provider_builder = provider_builder.with_config(config); } - let provider = provider_builder.build(); - let tracer = provider - .tracer_builder("opentelemetry-otlp") - .with_version(env!("CARGO_PKG_VERSION")) - .build(); - let _ = global::set_tracer_provider(provider); - tracer + + provider_builder.build() } fn build_batch_with_exporter( @@ -152,7 +144,7 @@ fn build_batch_with_exporter( trace_config: Option, runtime: R, batch_config: Option, -) -> sdk::trace::Tracer { +) -> sdk::trace::TracerProvider { let mut provider_builder = sdk::trace::TracerProvider::builder(); let batch_processor = sdk::trace::BatchSpanProcessor::builder(exporter, runtime) .with_batch_config(batch_config.unwrap_or_default()) @@ -162,13 +154,7 @@ fn build_batch_with_exporter( if let Some(config) = trace_config { provider_builder = provider_builder.with_config(config); } - let provider = provider_builder.build(); - let tracer = provider - .tracer_builder("opentelemetry-otlp") - .with_version(env!("CARGO_PKG_VERSION")) - .build(); - let _ = global::set_tracer_provider(provider); - tracer + provider_builder.build() } /// OTLP span exporter builder. diff --git a/opentelemetry-otlp/tests/integration_test/tests/traces.rs b/opentelemetry-otlp/tests/integration_test/tests/traces.rs index e68e0d36a7..0c357e26b8 100644 --- a/opentelemetry-otlp/tests/integration_test/tests/traces.rs +++ b/opentelemetry-otlp/tests/integration_test/tests/traces.rs @@ -13,7 +13,7 @@ use std::error::Error; use std::fs::File; use std::os::unix::fs::MetadataExt; -fn init_tracer() -> Result { +fn init_tracer_provider() -> Result { opentelemetry_otlp::new_pipeline() .tracing() .with_exporter(opentelemetry_otlp::new_exporter().tonic()) @@ -30,10 +30,8 @@ const LEMONS_KEY: Key = Key::from_static_str("lemons"); const ANOTHER_KEY: Key = Key::from_static_str("ex.com/another"); pub async fn traces() -> Result<(), Box> { - // By binding the result to an unused variable, the lifetime of the variable - // matches the containing block, reporting traces and metrics during the whole - // execution. - let _ = init_tracer()?; + let tracer_provider = init_tracer_provider().expect("Failed to initialize tracer provider."); + global::set_tracer_provider(tracer_provider.clone()); let tracer = global::tracer("ex.com/basic"); diff --git a/opentelemetry-otlp/tests/smoke.rs b/opentelemetry-otlp/tests/smoke.rs index ba2d69837c..c217f8f9d6 100644 --- a/opentelemetry-otlp/tests/smoke.rs +++ b/opentelemetry-otlp/tests/smoke.rs @@ -1,4 +1,5 @@ use futures_util::StreamExt; +use opentelemetry::global; use opentelemetry::global::shutdown_tracer_provider; use opentelemetry::trace::{Span, SpanKind, Tracer}; use opentelemetry_otlp::WithExportConfig; @@ -80,10 +81,10 @@ async fn smoke_tracer() { let (addr, mut req_rx) = setup().await; { - println!("Installing tracer..."); + println!("Installing tracer provider..."); let mut metadata = tonic::metadata::MetadataMap::new(); metadata.insert("x-header-key", "header-value".parse().unwrap()); - let tracer = opentelemetry_otlp::new_pipeline() + let tracer_provider = opentelemetry_otlp::new_pipeline() .tracing() .with_exporter( #[cfg(feature = "gzip-tonic")] @@ -101,6 +102,10 @@ async fn smoke_tracer() { .install_batch(opentelemetry_sdk::runtime::Tokio) .expect("failed to install"); + global::set_tracer_provider(tracer_provider); + + let tracer = global::tracer("smoke"); + println!("Sending span..."); let mut span = tracer .span_builder("my-test-span") From e699233bcc4e8e7e00d4963652538c4d7174f432 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Fri, 24 May 2024 14:44:21 -0700 Subject: [PATCH 13/13] Fix typos (#1828) Co-authored-by: Cijo Thomas --- opentelemetry-jaeger-propagator/src/propagator.rs | 2 +- opentelemetry-sdk/benches/metric_counter.rs | 12 ++++++------ opentelemetry-sdk/benches/metric_gauge.rs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/opentelemetry-jaeger-propagator/src/propagator.rs b/opentelemetry-jaeger-propagator/src/propagator.rs index 4f5491d932..4dbd0b16fc 100644 --- a/opentelemetry-jaeger-propagator/src/propagator.rs +++ b/opentelemetry-jaeger-propagator/src/propagator.rs @@ -138,7 +138,7 @@ impl Propagator { /// First bit controls whether to sample /// Second bit controls whether it's a debug trace /// Third bit is not used. - /// Forth bit is firehose flag, which is not supported in OT now. + /// Fourth bit is firehose flag, which is not supported in OT now. fn extract_trace_flags(&self, flag: &str) -> Result { if flag.len() > 2 { return Err(()); diff --git a/opentelemetry-sdk/benches/metric_counter.rs b/opentelemetry-sdk/benches/metric_counter.rs index 8962daa125..d3b7bb1a65 100644 --- a/opentelemetry-sdk/benches/metric_counter.rs +++ b/opentelemetry-sdk/benches/metric_counter.rs @@ -66,14 +66,14 @@ fn counter_add(c: &mut Criterion) { let index_first_attribute = rands[0]; let index_second_attribute = rands[1]; let index_third_attribute = rands[2]; - let index_forth_attribute = rands[3]; + 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_forth_attribute]), + KeyValue::new("attribute4", attribute_values[index_fourth_attribute]), ], ); }); @@ -94,14 +94,14 @@ fn counter_add(c: &mut Criterion) { let index_first_attribute = rands[0]; let index_second_attribute = rands[1]; let index_third_attribute = rands[2]; - let index_forth_attribute = rands[3]; + 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_forth_attribute]), + KeyValue::new("attribute4", attribute_values[index_fourth_attribute]), ], ); }); @@ -126,14 +126,14 @@ fn counter_add(c: &mut Criterion) { let index_first_attribute = rands[0]; let index_second_attribute = rands[1]; let index_third_attribute = rands[2]; - let index_forth_attribute = rands[3]; + 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_forth_attribute]), + KeyValue::new("attribute4", attribute_values[index_fourth_attribute]), ], ); }); diff --git a/opentelemetry-sdk/benches/metric_gauge.rs b/opentelemetry-sdk/benches/metric_gauge.rs index 3217480760..e455c5a577 100644 --- a/opentelemetry-sdk/benches/metric_gauge.rs +++ b/opentelemetry-sdk/benches/metric_gauge.rs @@ -63,14 +63,14 @@ fn gauge_record(c: &mut Criterion) { let index_first_attribute = rands[0]; let index_second_attribute = rands[1]; let index_third_attribute = rands[2]; - let index_forth_attribute = rands[3]; + let index_fourth_attribute = rands[3]; gauge.record( 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_forth_attribute]), + KeyValue::new("attribute4", attribute_values[index_fourth_attribute]), ], ); });