Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Metrics API] [Breaking] Enforce meter name, version schema_url to be static string slices #2112

Merged
9 changes: 6 additions & 3 deletions opentelemetry-sdk/benches/metric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,10 @@ fn bench_counter(view: Option<Box<dyn View>>, temporality: &str) -> (SharedReade
builder = builder.with_view(view);
}
let provider = builder.build();
let cntr = provider.meter("test").u64_counter("hello").init();
let cntr = provider
.meter("test".to_string())
.u64_counter("hello")
.init();

(rdr, cntr)
}
Expand Down Expand Up @@ -365,7 +368,7 @@ fn bench_histogram(bound_count: usize) -> (SharedReader, Histogram<u64>) {
if let Some(view) = view {
builder = builder.with_view(view);
}
let mtr = builder.build().meter("test_meter");
let mtr = builder.build().meter("test_meter".to_string());
let hist = mtr
.u64_histogram(format!("histogram_{}", bound_count))
.init();
Expand Down Expand Up @@ -405,7 +408,7 @@ fn benchmark_collect_histogram(b: &mut Bencher, n: usize) {
let mtr = SdkMeterProvider::builder()
.with_reader(r.clone())
.build()
.meter("sdk/metric/bench/histogram");
.meter("sdk/metric/bench/histogram".to_string());

for i in 0..n {
let h = mtr.u64_histogram(format!("fake_data_{i}")).init();
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/benches/metrics_counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn create_counter(name: &'static str) -> Counter<u64> {
let meter_provider: SdkMeterProvider = SdkMeterProvider::builder()
.with_reader(ManualReader::builder().build())
.build();
let meter = meter_provider.meter("benchmarks");
let meter = meter_provider.meter("benchmarks".to_string());

println!("Counter_Created");
meter.u64_counter(name).init()
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/benches/metrics_gauge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn create_gauge() -> Gauge<u64> {
let meter_provider: SdkMeterProvider = SdkMeterProvider::builder()
.with_reader(ManualReader::builder().build())
.build();
let meter = meter_provider.meter("benchmarks");
let meter = meter_provider.meter("benchmarks".to_string());

meter.u64_gauge("gauge_bench").init()
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/benches/metrics_histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn create_histogram(name: &'static str) -> Histogram<u64> {
let meter_provider: SdkMeterProvider = SdkMeterProvider::builder()
.with_reader(ManualReader::builder().build())
.build();
let meter = meter_provider.meter("benchmarks");
let meter = meter_provider.meter("benchmarks".to_string());

meter.u64_histogram(name).init()
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/metrics/meter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@
#[ignore = "See issue https://github.com/open-telemetry/opentelemetry-rust/issues/1699"]
fn test_instrument_creation() {
let provider = SdkMeterProvider::builder().build();
let meter = provider.meter("test");
let meter = provider.meter("test".to_string());

Check warning on line 543 in opentelemetry-sdk/src/metrics/meter.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/meter.rs#L543

Added line #L543 was not covered by tests
assert!(meter.u64_counter("test").try_init().is_ok());
let result = meter.u64_counter("test with invalid name").try_init();
// this assert fails, as result is always ok variant.
Expand Down
59 changes: 41 additions & 18 deletions opentelemetry-sdk/src/metrics/meter_provider.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use core::fmt;
use std::{
borrow::Cow,
collections::HashMap,
sync::{
atomic::{AtomicBool, Ordering},
Expand Down Expand Up @@ -139,9 +138,9 @@ impl Drop for SdkMeterProviderInner {
impl MeterProvider for SdkMeterProvider {
fn versioned_meter(
&self,
name: impl Into<Cow<'static, str>>,
version: Option<impl Into<Cow<'static, str>>>,
schema_url: Option<impl Into<Cow<'static, str>>>,
name: String,
version: Option<String>,
schema_url: Option<String>,
attributes: Option<Vec<KeyValue>>,
) -> Meter {
if self.inner.is_shutdown.load(Ordering::Relaxed) {
Expand Down Expand Up @@ -443,24 +442,48 @@ mod tests {
#[test]
fn same_meter_reused_same_scope() {
let provider = super::SdkMeterProvider::builder().build();
let _meter1 = provider.meter("test");
let _meter2 = provider.meter("test");
let _meter1 = provider.meter("test".to_string());
let _meter2 = provider.meter("test".to_string());
assert_eq!(provider.inner.meters.lock().unwrap().len(), 1);
let _meter3 =
provider.versioned_meter("test", Some("1.0.0"), Some("http://example.com"), None);
let _meter4 =
provider.versioned_meter("test", Some("1.0.0"), Some("http://example.com"), None);
let _meter5 =
provider.versioned_meter("test", Some("1.0.0"), Some("http://example.com"), None);
let _meter3 = provider.versioned_meter(
"test".to_string(),
Some("1.0.0".to_string()),
Some("http://example.com".to_string()),
None,
);
let _meter4 = provider.versioned_meter(
"test".to_string(),
Some("1.0.0".to_string()),
Some("http://example.com".to_string()),
None,
);
let _meter5 = provider.versioned_meter(
"test".to_string(),
Some("1.0.0".to_string()),
Some("http://example.com".to_string()),
None,
);
assert_eq!(provider.inner.meters.lock().unwrap().len(), 2);

// the below are different meters, as meter names are case sensitive
let _meter6 =
provider.versioned_meter("ABC", Some("1.0.0"), Some("http://example.com"), None);
let _meter7 =
provider.versioned_meter("Abc", Some("1.0.0"), Some("http://example.com"), None);
let _meter8 =
provider.versioned_meter("abc", Some("1.0.0"), Some("http://example.com"), None);
let _meter6 = provider.versioned_meter(
"ABC".to_string(),
Some("1.0.0".to_string()),
Some("http://example.com".to_string()),
None,
);
let _meter7 = provider.versioned_meter(
"Abc".to_string(),
Some("1.0.0".to_string()),
Some("http://example.com".to_string()),
None,
);
let _meter8 = provider.versioned_meter(
"abc".to_string(),
Some("1.0.0".to_string()),
Some("http://example.com".to_string()),
None,
);
assert_eq!(provider.inner.meters.lock().unwrap().len(), 5);
}
}
30 changes: 15 additions & 15 deletions opentelemetry-sdk/src/metrics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@
let meter_provider = SdkMeterProvider::builder().with_reader(reader).build();

// Act
let meter = meter_provider.meter("test");
let meter = meter_provider.meter("test".to_string());
let counter = meter
.u64_counter("my_counter")
.with_unit("my_unit")
Expand Down Expand Up @@ -483,8 +483,8 @@
let meter_provider = SdkMeterProvider::builder().with_reader(reader).build();

// Act
let meter1 = meter_provider.meter("test.meter1");
let meter2 = meter_provider.meter("test.meter2");
let meter1 = meter_provider.meter("test.meter1".to_string());
let meter2 = meter_provider.meter("test.meter2".to_string());
let counter1 = meter1
.u64_counter("my_counter")
.with_unit("my_unit")
Expand Down Expand Up @@ -575,15 +575,15 @@
// Meters are identical except for scope attributes, but scope attributes are not an identifying property.
// Hence there should be a single metric stream output for this test.
let meter1 = meter_provider.versioned_meter(
"test.meter",
Some("v0.1.0"),
Some("schema_url"),
"test.meter".to_string(),
Some("v0.1.0".to_string()),
Some("schema_url".to_string()),
Some(vec![KeyValue::new("key", "value1")]),
);
let meter2 = meter_provider.versioned_meter(
"test.meter",
Some("v0.1.0"),
Some("schema_url"),
"test.meter".to_string(),
Some("v0.1.0".to_string()),
Some("schema_url".to_string()),
Some(vec![KeyValue::new("key", "value2")]),
);
let counter1 = meter1
Expand Down Expand Up @@ -669,7 +669,7 @@
.build();

// Act
let meter = meter_provider.meter("test");
let meter = meter_provider.meter("test".to_string());
let histogram = meter
.f64_histogram("test_histogram")
.with_unit("test_unit")
Expand Down Expand Up @@ -714,7 +714,7 @@
.build();

// Act
let meter = meter_provider.meter("test");
let meter = meter_provider.meter("test".to_string());

Check warning on line 717 in opentelemetry-sdk/src/metrics/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-sdk/src/metrics/mod.rs#L717

Added line #L717 was not covered by tests
let _observable_counter = meter
.u64_observable_counter("my_observable_counter")
.with_callback(|observer| {
Expand Down Expand Up @@ -789,7 +789,7 @@
.build();

// Act
let meter = meter_provider.meter("test");
let meter = meter_provider.meter("test".to_string());
let counter = meter.u64_counter("my_counter").init();

// Normally, this would generate 3 time-series, but since the view
Expand Down Expand Up @@ -2276,7 +2276,7 @@
counter_name: &'static str,
unit: Option<&'static str>,
) -> Counter<u64> {
let meter = self.meter_provider.meter(meter_name);
let meter = self.meter_provider.meter(meter_name.to_string());
let mut counter_builder = meter.u64_counter(counter_name);
if let Some(unit_name) = unit {
counter_builder = counter_builder.with_unit(unit_name);
Expand All @@ -2290,7 +2290,7 @@
counter_name: &'static str,
unit: Option<&'static str>,
) -> UpDownCounter<i64> {
let meter = self.meter_provider.meter(meter_name);
let meter = self.meter_provider.meter(meter_name.to_string());
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_name);
Expand All @@ -2299,7 +2299,7 @@
}

fn meter(&self) -> Meter {
self.meter_provider.meter("test")
self.meter_provider.meter("test".to_string())
}

fn flush_metrics(&self) {
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/metrics/periodic_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ mod tests {

// Act
let meter_provider = SdkMeterProvider::builder().with_reader(reader).build();
let meter = meter_provider.meter("test");
let meter = meter_provider.meter("test".to_string());
let _counter = meter
.u64_observable_counter("testcounter")
.with_callback(move |_| {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use std::sync::{Arc, Mutex};
/// .build();
///
/// // Create and record metrics using the MeterProvider
/// let meter = meter_provider.meter(std::borrow::Cow::Borrowed("example"));
/// let meter = meter_provider.meter("example".to_string());
/// let counter = meter.u64_counter("my_counter").init();
/// counter.add(1, &[KeyValue::new("key", "value")]);
///
Expand Down
Loading
Loading