From 3976f3d4867d564c98a566df83eb59e31c653b84 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:26:30 -0700 Subject: [PATCH] [Breaking] Enforce meter name, version schema_url to be static string slices (#2112) Co-authored-by: Cijo Thomas Co-authored-by: Lalit Kumar Bhasin Co-authored-by: Zhongyang Wu --- .../src/metrics/meter_provider.rs | 7 +- .../src/testing/metrics/in_memory_exporter.rs | 2 +- opentelemetry/src/global/metrics.rs | 111 +++--------------- opentelemetry/src/metrics/meter.rs | 15 +-- opentelemetry/src/metrics/noop.rs | 8 +- 5 files changed, 27 insertions(+), 116 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 4546f0d2e2..9cd798eb44 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -1,6 +1,5 @@ use core::fmt; use std::{ - borrow::Cow, collections::HashMap, sync::{ atomic::{AtomicBool, Ordering}, @@ -139,9 +138,9 @@ impl Drop for SdkMeterProviderInner { impl MeterProvider for SdkMeterProvider { fn versioned_meter( &self, - name: impl Into>, - version: Option>>, - schema_url: Option>>, + name: &'static str, + version: Option<&'static str>, + schema_url: Option<&'static str>, attributes: Option>, ) -> Meter { if self.inner.is_shutdown.load(Ordering::Relaxed) { diff --git a/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs b/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs index 3f85b360b7..89eec6e6b7 100644 --- a/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs +++ b/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs @@ -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"); /// let counter = meter.u64_counter("my_counter").init(); /// counter.add(1, &[KeyValue::new("key", "value")]); /// diff --git a/opentelemetry/src/global/metrics.rs b/opentelemetry/src/global/metrics.rs index 7826f9920e..630e9e2ba7 100644 --- a/opentelemetry/src/global/metrics.rs +++ b/opentelemetry/src/global/metrics.rs @@ -1,90 +1,13 @@ use crate::metrics::{self, Meter, MeterProvider}; use crate::KeyValue; -use core::fmt; use once_cell::sync::Lazy; -use std::{ - borrow::Cow, - sync::{Arc, RwLock}, -}; +use std::sync::{Arc, RwLock}; -/// The global `MeterProvider` singleton. -static GLOBAL_METER_PROVIDER: Lazy> = Lazy::new(|| { - RwLock::new(GlobalMeterProvider::new( - metrics::noop::NoopMeterProvider::new(), - )) -}); - -/// Allows a specific [MeterProvider] to be used generically by the -/// [GlobalMeterProvider] by mirroring the interface and boxing the return types. -trait ObjectSafeMeterProvider { - /// Creates a versioned named meter instance that is a trait object through the underlying - /// [MeterProvider]. - fn versioned_meter_cow( - &self, - name: Cow<'static, str>, - version: Option>, - schema_url: Option>, - attributes: Option>, - ) -> Meter; -} - -impl

ObjectSafeMeterProvider for P -where - P: MeterProvider, -{ - /// Return a versioned boxed tracer - fn versioned_meter_cow( - &self, - name: Cow<'static, str>, - version: Option>, - schema_url: Option>, - attributes: Option>, - ) -> Meter { - self.versioned_meter(name, version, schema_url, attributes) - } -} +type GlobalMeterProvider = Arc; -/// Represents the globally configured [`MeterProvider`] instance for this -/// application. -#[derive(Clone)] -pub struct GlobalMeterProvider { - provider: Arc, -} - -impl fmt::Debug for GlobalMeterProvider { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("GlobalMeterProvider").finish() - } -} - -impl MeterProvider for GlobalMeterProvider { - fn versioned_meter( - &self, - name: impl Into>, - version: Option>>, - schema_url: Option>>, - attributes: Option>, - ) -> Meter { - self.provider.versioned_meter_cow( - name.into(), - version.map(Into::into), - schema_url.map(Into::into), - attributes, - ) - } -} - -impl GlobalMeterProvider { - /// Create a new global meter provider - fn new

(provider: P) -> Self - where - P: MeterProvider + Send + Sync + 'static, - { - GlobalMeterProvider { - provider: Arc::new(provider), - } - } -} +/// The global `MeterProvider` singleton. +static GLOBAL_METER_PROVIDER: Lazy> = + Lazy::new(|| RwLock::new(Arc::new(metrics::noop::NoopMeterProvider::new()))); /// Sets the given [`MeterProvider`] instance as the current global meter /// provider. @@ -95,11 +18,10 @@ where let mut global_provider = GLOBAL_METER_PROVIDER .write() .expect("GLOBAL_METER_PROVIDER RwLock poisoned"); - *global_provider = GlobalMeterProvider::new(new_provider); + *global_provider = Arc::new(new_provider); } -/// Returns an instance of the currently configured global [`MeterProvider`] -/// through [`GlobalMeterProvider`]. +/// Returns an instance of the currently configured global [`MeterProvider`]. pub fn meter_provider() -> GlobalMeterProvider { GLOBAL_METER_PROVIDER .read() @@ -107,13 +29,13 @@ pub fn meter_provider() -> GlobalMeterProvider { .clone() } -/// Creates a named [`Meter`] via the configured [`GlobalMeterProvider`]. +/// Creates a named [`Meter`] via the currently configured global [`MeterProvider`]. /// /// If the name is an empty string, the provider will use a default name. /// /// This is a more convenient way of expressing `global::meter_provider().meter(name)`. -pub fn meter(name: impl Into>) -> Meter { - meter_provider().meter(name.into()) +pub fn meter(name: &'static str) -> Meter { + meter_provider().meter(name) } /// Creates a [`Meter`] with the name, version and schema url. @@ -138,15 +60,10 @@ pub fn meter(name: impl Into>) -> Meter { /// ); /// ``` pub fn meter_with_version( - name: impl Into>, - version: Option>>, - schema_url: Option>>, + name: &'static str, + version: Option<&'static str>, + schema_url: Option<&'static str>, attributes: Option>, ) -> Meter { - meter_provider().versioned_meter( - name.into(), - version.map(Into::into), - schema_url.map(Into::into), - attributes, - ) + meter_provider().versioned_meter(name, version, schema_url, attributes) } diff --git a/opentelemetry/src/metrics/meter.rs b/opentelemetry/src/metrics/meter.rs index 3422240b48..b70291b916 100644 --- a/opentelemetry/src/metrics/meter.rs +++ b/opentelemetry/src/metrics/meter.rs @@ -39,13 +39,8 @@ pub trait MeterProvider { /// Some(vec![KeyValue::new("key", "value")]), /// ); /// ``` - fn meter(&self, name: impl Into>) -> Meter { - self.versioned_meter( - name, - None::>, - None::>, - None, - ) + fn meter(&self, name: &'static str) -> Meter { + self.versioned_meter(name, None, None, None) } /// Returns a new versioned meter with a given name. @@ -56,9 +51,9 @@ pub trait MeterProvider { /// default name will be used instead. fn versioned_meter( &self, - name: impl Into>, - version: Option>>, - schema_url: Option>>, + name: &'static str, + version: Option<&'static str>, + schema_url: Option<&'static str>, attributes: Option>, ) -> Meter; } diff --git a/opentelemetry/src/metrics/noop.rs b/opentelemetry/src/metrics/noop.rs index 716e4ca3c8..9db39890aa 100644 --- a/opentelemetry/src/metrics/noop.rs +++ b/opentelemetry/src/metrics/noop.rs @@ -10,7 +10,7 @@ use crate::{ }, KeyValue, }; -use std::{any::Any, borrow::Cow, sync::Arc}; +use std::{any::Any, sync::Arc}; /// A no-op instance of a `MetricProvider` #[derive(Debug, Default)] @@ -28,9 +28,9 @@ impl NoopMeterProvider { impl MeterProvider for NoopMeterProvider { fn versioned_meter( &self, - _name: impl Into>, - _version: Option>>, - _schema_url: Option>>, + _name: &'static str, + _version: Option<&'static str>, + _schema_url: Option<&'static str>, _attributes: Option>, ) -> Meter { Meter::new(Arc::new(NoopMeterCore::new()))