From 0f016ce0e79369d76c4eea8949caae7e50613b99 Mon Sep 17 00:00:00 2001 From: Mindaugas Vinkelis <fraillt@gmail.com> Date: Mon, 9 Sep 2024 22:58:03 +0300 Subject: [PATCH] AggregationSelector is not needed --- opentelemetry-otlp/src/exporter/http/mod.rs | 10 +-- opentelemetry-otlp/src/exporter/tonic/mod.rs | 10 +-- opentelemetry-otlp/src/lib.rs | 3 +- opentelemetry-otlp/src/metric.rs | 35 ++-------- .../expected/serialized_traces.json | 6 ++ opentelemetry-sdk/benches/metric.rs | 8 +-- opentelemetry-sdk/src/metrics/aggregation.rs | 4 +- opentelemetry-sdk/src/metrics/exporter.rs | 9 +-- .../src/metrics/manual_reader.rs | 34 +--------- .../src/metrics/periodic_reader.rs | 9 +-- opentelemetry-sdk/src/metrics/pipeline.rs | 45 ++++++++++--- opentelemetry-sdk/src/metrics/reader.rs | 67 +------------------ .../src/testing/metrics/in_memory_exporter.rs | 29 +------- .../src/testing/metrics/metric_reader.rs | 9 +-- opentelemetry-stdout/src/metrics/exporter.rs | 27 +------- 15 files changed, 66 insertions(+), 239 deletions(-) diff --git a/opentelemetry-otlp/src/exporter/http/mod.rs b/opentelemetry-otlp/src/exporter/http/mod.rs index 3ccefa4caa..9110b0c474 100644 --- a/opentelemetry-otlp/src/exporter/http/mod.rs +++ b/opentelemetry-otlp/src/exporter/http/mod.rs @@ -78,7 +78,7 @@ impl Default for HttpConfig { /// ``` /// # #[cfg(feature="metrics")] /// use opentelemetry_sdk::metrics::reader::{ -/// DefaultAggregationSelector, DefaultTemporalitySelector, +/// DefaultTemporalitySelector, /// }; /// /// # fn main() -> Result<(), Box<dyn std::error::Error>> { @@ -91,7 +91,6 @@ impl Default for HttpConfig { /// let metrics_exporter = opentelemetry_otlp::new_exporter() /// .http() /// .build_metrics_exporter( -/// Box::new(DefaultAggregationSelector::new()), /// Box::new(DefaultTemporalitySelector::new()), /// )?; /// @@ -252,7 +251,6 @@ impl HttpExporterBuilder { #[cfg(feature = "metrics")] pub fn build_metrics_exporter( mut self, - aggregation_selector: Box<dyn opentelemetry_sdk::metrics::reader::AggregationSelector>, temporality_selector: Box<dyn opentelemetry_sdk::metrics::reader::TemporalitySelector>, ) -> opentelemetry::metrics::Result<crate::MetricsExporter> { use crate::{ @@ -267,11 +265,7 @@ impl HttpExporterBuilder { OTEL_EXPORTER_OTLP_METRICS_HEADERS, )?; - Ok(crate::MetricsExporter::new( - client, - temporality_selector, - aggregation_selector, - )) + Ok(crate::MetricsExporter::new(client, temporality_selector)) } } diff --git a/opentelemetry-otlp/src/exporter/tonic/mod.rs b/opentelemetry-otlp/src/exporter/tonic/mod.rs index 69ecb4fe79..af5a5ca7de 100644 --- a/opentelemetry-otlp/src/exporter/tonic/mod.rs +++ b/opentelemetry-otlp/src/exporter/tonic/mod.rs @@ -97,7 +97,7 @@ fn resolve_compression( /// ```no_run /// # #[cfg(feature="metrics")] /// use opentelemetry_sdk::metrics::reader::{ -/// DefaultAggregationSelector, DefaultTemporalitySelector, +/// DefaultTemporalitySelector, /// }; /// /// # fn main() -> Result<(), Box<dyn std::error::Error>> { @@ -110,7 +110,6 @@ fn resolve_compression( /// let metrics_exporter = opentelemetry_otlp::new_exporter() /// .tonic() /// .build_metrics_exporter( -/// Box::new(DefaultAggregationSelector::new()), /// Box::new(DefaultTemporalitySelector::new()), /// )?; /// @@ -332,7 +331,6 @@ impl TonicExporterBuilder { #[cfg(feature = "metrics")] pub fn build_metrics_exporter( self, - aggregation_selector: Box<dyn opentelemetry_sdk::metrics::reader::AggregationSelector>, temporality_selector: Box<dyn opentelemetry_sdk::metrics::reader::TemporalitySelector>, ) -> opentelemetry::metrics::Result<crate::MetricsExporter> { use crate::MetricsExporter; @@ -347,11 +345,7 @@ impl TonicExporterBuilder { let client = TonicMetricsClient::new(channel, interceptor, compression); - Ok(MetricsExporter::new( - client, - temporality_selector, - aggregation_selector, - )) + Ok(MetricsExporter::new(client, temporality_selector)) } /// Build a new tonic span exporter diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 7326bc6ac3..1aad8f8677 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -126,7 +126,7 @@ //! use opentelemetry::{global, KeyValue, trace::Tracer}; //! use opentelemetry_sdk::{trace::{self, RandomIdGenerator, Sampler}, Resource}; //! # #[cfg(feature = "metrics")] -//! use opentelemetry_sdk::metrics::reader::{DefaultAggregationSelector, DefaultTemporalitySelector}; +//! use opentelemetry_sdk::metrics::reader::DefaultTemporalitySelector; //! use opentelemetry_otlp::{Protocol, WithExportConfig, ExportConfig}; //! use std::time::Duration; //! # #[cfg(feature = "grpc-tonic")] @@ -184,7 +184,6 @@ //! .with_resource(Resource::new(vec![KeyValue::new("service.name", "example")])) //! .with_period(Duration::from_secs(3)) //! .with_timeout(Duration::from_secs(10)) -//! .with_aggregation_selector(DefaultAggregationSelector::new()) //! .with_temporality_selector(DefaultTemporalitySelector::new()) //! .build(); //! # } diff --git a/opentelemetry-otlp/src/metric.rs b/opentelemetry-otlp/src/metric.rs index 82b874cdd3..83474e94ff 100644 --- a/opentelemetry-otlp/src/metric.rs +++ b/opentelemetry-otlp/src/metric.rs @@ -14,11 +14,8 @@ use opentelemetry_sdk::{ metrics::{ data::{ResourceMetrics, Temporality}, exporter::PushMetricsExporter, - reader::{ - AggregationSelector, DefaultAggregationSelector, DefaultTemporalitySelector, - TemporalitySelector, - }, - Aggregation, InstrumentKind, PeriodicReader, SdkMeterProvider, + reader::{DefaultTemporalitySelector, TemporalitySelector}, + InstrumentKind, PeriodicReader, SdkMeterProvider, }, runtime::Runtime, Resource, @@ -50,7 +47,6 @@ impl OtlpPipeline { { OtlpMetricPipeline { rt, - aggregator_selector: None, temporality_selector: None, exporter_pipeline: NoExporterConfig(()), resource: None, @@ -82,21 +78,19 @@ impl MetricsExporterBuilder { pub fn build_metrics_exporter( self, temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, ) -> Result<MetricsExporter> { match self { #[cfg(feature = "grpc-tonic")] MetricsExporterBuilder::Tonic(builder) => { - builder.build_metrics_exporter(aggregation_selector, temporality_selector) + builder.build_metrics_exporter(temporality_selector) } #[cfg(feature = "http-proto")] MetricsExporterBuilder::Http(builder) => { - builder.build_metrics_exporter(aggregation_selector, temporality_selector) + builder.build_metrics_exporter(temporality_selector) } #[cfg(not(any(feature = "http-proto", feature = "grpc-tonic")))] MetricsExporterBuilder::Unconfigured => { drop(temporality_selector); - drop(aggregation_selector); Err(opentelemetry::metrics::MetricsError::Other( "no configured metrics exporter, enable `http-proto` or `grpc-tonic` feature to configure a metrics exporter".into(), )) @@ -125,7 +119,6 @@ impl From<HttpExporterBuilder> for MetricsExporterBuilder { /// runtime. pub struct OtlpMetricPipeline<RT, EB> { rt: RT, - aggregator_selector: Option<Box<dyn AggregationSelector>>, temporality_selector: Option<Box<dyn TemporalitySelector>>, exporter_pipeline: EB, resource: Option<Resource>, @@ -178,14 +171,6 @@ where pub fn with_delta_temporality(self) -> Self { self.with_temporality_selector(DeltaTemporalitySelector) } - - /// Build with the given aggregation selector - pub fn with_aggregation_selector<T: AggregationSelector + 'static>(self, selector: T) -> Self { - OtlpMetricPipeline { - aggregator_selector: Some(Box::new(selector)), - ..self - } - } } impl<RT> OtlpMetricPipeline<RT, NoExporterConfig> @@ -200,7 +185,6 @@ where OtlpMetricPipeline { exporter_pipeline: pipeline.into(), rt: self.rt, - aggregator_selector: self.aggregator_selector, temporality_selector: self.temporality_selector, resource: self.resource, period: self.period, @@ -218,8 +202,6 @@ where let exporter = self.exporter_pipeline.build_metrics_exporter( self.temporality_selector .unwrap_or_else(|| Box::new(DefaultTemporalitySelector::new())), - self.aggregator_selector - .unwrap_or_else(|| Box::new(DefaultAggregationSelector::new())), )?; let mut builder = PeriodicReader::builder(exporter, self.rt); @@ -295,7 +277,6 @@ pub trait MetricsClient: fmt::Debug + Send + Sync + 'static { pub struct MetricsExporter { client: Box<dyn MetricsClient>, temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, } impl Debug for MetricsExporter { @@ -310,12 +291,6 @@ impl TemporalitySelector for MetricsExporter { } } -impl AggregationSelector for MetricsExporter { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - self.aggregation_selector.aggregation(kind) - } -} - #[async_trait] impl PushMetricsExporter for MetricsExporter { async fn export(&self, metrics: &mut ResourceMetrics) -> Result<()> { @@ -337,12 +312,10 @@ impl MetricsExporter { pub fn new( client: impl MetricsClient, temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, ) -> MetricsExporter { MetricsExporter { client: Box::new(client), temporality_selector, - aggregation_selector, } } } diff --git a/opentelemetry-otlp/tests/integration_test/expected/serialized_traces.json b/opentelemetry-otlp/tests/integration_test/expected/serialized_traces.json index 849e66dd7b..e5982877cf 100644 --- a/opentelemetry-otlp/tests/integration_test/expected/serialized_traces.json +++ b/opentelemetry-otlp/tests/integration_test/expected/serialized_traces.json @@ -112,6 +112,12 @@ "value": { "intValue": "100" } + }, + { + "key": "number/int", + "value": { + "intValue": "100" + } } ], "droppedAttributesCount": 0 diff --git a/opentelemetry-sdk/benches/metric.rs b/opentelemetry-sdk/benches/metric.rs index d018634e04..88143bccff 100644 --- a/opentelemetry-sdk/benches/metric.rs +++ b/opentelemetry-sdk/benches/metric.rs @@ -10,7 +10,7 @@ use opentelemetry_sdk::{ metrics::{ data::{ResourceMetrics, Temporality}, new_view, - reader::{AggregationSelector, MetricReader, TemporalitySelector}, + reader::{MetricReader, TemporalitySelector}, Aggregation, Instrument, InstrumentKind, ManualReader, Pipeline, SdkMeterProvider, Stream, View, }, @@ -26,12 +26,6 @@ impl TemporalitySelector for SharedReader { } } -impl AggregationSelector for SharedReader { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - self.0.aggregation(kind) - } -} - impl MetricReader for SharedReader { fn register_pipeline(&self, pipeline: Weak<Pipeline>) { self.0.register_pipeline(pipeline) diff --git a/opentelemetry-sdk/src/metrics/aggregation.rs b/opentelemetry-sdk/src/metrics/aggregation.rs index db2bee92d5..561aa00c4d 100644 --- a/opentelemetry-sdk/src/metrics/aggregation.rs +++ b/opentelemetry-sdk/src/metrics/aggregation.rs @@ -17,10 +17,10 @@ pub enum Aggregation { /// instrument kind that differs from the default. This aggregation ensures the /// default is used. /// - /// See the [DefaultAggregationSelector] for information about the default + /// See the [the spec] for information about the default /// instrument kind selection mapping. /// - /// [DefaultAggregationSelector]: crate::metrics::reader::DefaultAggregationSelector + /// [the spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.19.0/specification/metrics/sdk.md#default-aggregation Default, /// An aggregation that summarizes a set of measurements as their arithmetic diff --git a/opentelemetry-sdk/src/metrics/exporter.rs b/opentelemetry-sdk/src/metrics/exporter.rs index fbe8003fa2..c49aaa75dd 100644 --- a/opentelemetry-sdk/src/metrics/exporter.rs +++ b/opentelemetry-sdk/src/metrics/exporter.rs @@ -3,18 +3,13 @@ use async_trait::async_trait; use opentelemetry::metrics::Result; -use crate::metrics::{ - data::ResourceMetrics, - reader::{AggregationSelector, TemporalitySelector}, -}; +use crate::metrics::{data::ResourceMetrics, reader::TemporalitySelector}; /// Exporter handles the delivery of metric data to external receivers. /// /// This is the final component in the metric push pipeline. #[async_trait] -pub trait PushMetricsExporter: - AggregationSelector + TemporalitySelector + Send + Sync + 'static -{ +pub trait PushMetricsExporter: TemporalitySelector + Send + Sync + 'static { /// Export serializes and transmits metric data to a receiver. /// /// All retry logic must be contained in this function. The SDK does not diff --git a/opentelemetry-sdk/src/metrics/manual_reader.rs b/opentelemetry-sdk/src/metrics/manual_reader.rs index d52fc1e96e..0d1000ad35 100644 --- a/opentelemetry-sdk/src/metrics/manual_reader.rs +++ b/opentelemetry-sdk/src/metrics/manual_reader.rs @@ -13,8 +13,7 @@ use super::{ instrument::InstrumentKind, pipeline::Pipeline, reader::{ - AggregationSelector, DefaultAggregationSelector, DefaultTemporalitySelector, - MetricProducer, MetricReader, SdkProducer, TemporalitySelector, + DefaultTemporalitySelector, MetricProducer, MetricReader, SdkProducer, TemporalitySelector, }, }; @@ -34,7 +33,6 @@ use super::{ pub struct ManualReader { inner: Box<Mutex<ManualReaderInner>>, temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, } impl Default for ManualReader { @@ -65,7 +63,6 @@ impl ManualReader { /// A [MetricReader] which is directly called to collect metrics. pub(crate) fn new( temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, producers: Vec<Box<dyn MetricProducer>>, ) -> Self { ManualReader { @@ -75,7 +72,6 @@ impl ManualReader { external_producers: producers, })), temporality_selector, - aggregation_selector, } } } @@ -86,12 +82,6 @@ impl TemporalitySelector for ManualReader { } } -impl AggregationSelector for ManualReader { - fn aggregation(&self, kind: InstrumentKind) -> super::aggregation::Aggregation { - self.aggregation_selector.aggregation(kind) - } -} - impl MetricReader for ManualReader { /// Register a pipeline which enables the caller to read metrics from the SDK /// on demand. @@ -159,7 +149,6 @@ impl MetricReader for ManualReader { /// Configuration for a [ManualReader] pub struct ManualReaderBuilder { temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, producers: Vec<Box<dyn MetricProducer>>, } @@ -173,7 +162,6 @@ impl Default for ManualReaderBuilder { fn default() -> Self { ManualReaderBuilder { temporality_selector: Box::new(DefaultTemporalitySelector { _private: () }), - aggregation_selector: Box::new(DefaultAggregationSelector { _private: () }), producers: vec![], } } @@ -196,20 +184,6 @@ impl ManualReaderBuilder { self } - /// Sets the [AggregationSelector] a reader will use to determine the - /// aggregation to use for an instrument based on its kind. - /// - /// If this option is not used, the reader will use the default aggregation - /// selector or the aggregation explicitly passed for a view matching an - /// instrument. - pub fn with_aggregation_selector( - mut self, - aggregation_selector: impl AggregationSelector + 'static, - ) -> Self { - self.aggregation_selector = Box::new(aggregation_selector); - self - } - /// Registers a an external [MetricProducer] with this reader. /// /// The producer is used as a source of aggregated metric data which is @@ -221,10 +195,6 @@ impl ManualReaderBuilder { /// Create a new [ManualReader] from this configuration. pub fn build(self) -> ManualReader { - ManualReader::new( - self.temporality_selector, - self.aggregation_selector, - self.producers, - ) + ManualReader::new(self.temporality_selector, self.producers) } } diff --git a/opentelemetry-sdk/src/metrics/periodic_reader.rs b/opentelemetry-sdk/src/metrics/periodic_reader.rs index 034053bdbc..b664f4014e 100644 --- a/opentelemetry-sdk/src/metrics/periodic_reader.rs +++ b/opentelemetry-sdk/src/metrics/periodic_reader.rs @@ -26,10 +26,9 @@ use crate::{ }; use super::{ - aggregation::Aggregation, data::{ResourceMetrics, Temporality}, instrument::InstrumentKind, - reader::{AggregationSelector, MetricReader, TemporalitySelector}, + reader::{MetricReader, TemporalitySelector}, Pipeline, }; @@ -300,12 +299,6 @@ impl<RT: Runtime> PeriodicReaderWorker<RT> { } } -impl AggregationSelector for PeriodicReader { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - self.exporter.aggregation(kind) - } -} - impl TemporalitySelector for PeriodicReader { fn temporality(&self, kind: InstrumentKind) -> Temporality { self.exporter.temporality(kind) diff --git a/opentelemetry-sdk/src/metrics/pipeline.rs b/opentelemetry-sdk/src/metrics/pipeline.rs index c55b407638..28981df101 100644 --- a/opentelemetry-sdk/src/metrics/pipeline.rs +++ b/opentelemetry-sdk/src/metrics/pipeline.rs @@ -20,12 +20,14 @@ use crate::{ internal, internal::AggregateBuilder, internal::Number, - reader::{AggregationSelector, DefaultAggregationSelector, MetricReader, SdkProducer}, + reader::{MetricReader, SdkProducer}, view::View, }, Resource, }; +use super::Aggregation; + /// Connects all of the instruments created by a meter provider to a [MetricReader]. /// /// This is the object that will be registered when a meter provider is @@ -340,11 +342,11 @@ where let mut agg = stream .aggregation .take() - .unwrap_or_else(|| self.pipeline.reader.aggregation(kind)); + .unwrap_or_else(|| default_aggregation_selector(kind)); // Apply default if stream or reader aggregation returns default if matches!(agg, aggregation::Aggregation::Default) { - agg = DefaultAggregationSelector::new().aggregation(kind); + agg = default_aggregation_selector(kind); } if let Err(err) = is_aggregator_compatible(&kind, &agg) { @@ -430,6 +432,37 @@ where } } +/// The default aggregation and parameters for an instrument of [InstrumentKind]. +/// +/// This aggregation selector uses the following selection mapping per [the spec]: +/// +/// * Counter ⇨ Sum +/// * Observable Counter ⇨ Sum +/// * UpDownCounter ⇨ Sum +/// * Observable UpDownCounter ⇨ Sum +/// * Gauge ⇨ LastValue +/// * Observable Gauge ⇨ LastValue +/// * Histogram ⇨ ExplicitBucketHistogram +/// +/// [the spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.19.0/specification/metrics/sdk.md#default-aggregation +fn default_aggregation_selector(kind: InstrumentKind) -> Aggregation { + match kind { + InstrumentKind::Counter + | InstrumentKind::UpDownCounter + | InstrumentKind::ObservableCounter + | InstrumentKind::ObservableUpDownCounter => Aggregation::Sum, + InstrumentKind::Gauge => Aggregation::LastValue, + InstrumentKind::ObservableGauge => Aggregation::LastValue, + InstrumentKind::Histogram => Aggregation::ExplicitBucketHistogram { + boundaries: vec![ + 0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 750.0, 1000.0, 2500.0, + 5000.0, 7500.0, 10000.0, + ], + record_min_max: true, + }, + } +} + type AggregateFns<T> = ( Arc<dyn internal::Measure<T>>, Box<dyn internal::ComputeAggregation>, @@ -454,11 +487,7 @@ fn aggregate_fn<T: Number<T>>( } match agg { - Aggregation::Default => aggregate_fn( - b, - &DefaultAggregationSelector::new().aggregation(kind), - kind, - ), + Aggregation::Default => aggregate_fn(b, &default_aggregation_selector(kind), kind), Aggregation::Drop => Ok(None), Aggregation::LastValue => Ok(Some(box_val(b.last_value()))), Aggregation::Sum => { diff --git a/opentelemetry-sdk/src/metrics/reader.rs b/opentelemetry-sdk/src/metrics/reader.rs index f53e507dc1..ec54affe3d 100644 --- a/opentelemetry-sdk/src/metrics/reader.rs +++ b/opentelemetry-sdk/src/metrics/reader.rs @@ -4,7 +4,6 @@ use std::{fmt, sync::Weak}; use opentelemetry::metrics::Result; use super::{ - aggregation::Aggregation, data::{ResourceMetrics, ScopeMetrics, Temporality}, instrument::InstrumentKind, pipeline::Pipeline, @@ -24,9 +23,7 @@ use super::{ /// /// Pull-based exporters will typically implement `MetricReader` themselves, /// since they read on demand. -pub trait MetricReader: - AggregationSelector + TemporalitySelector + fmt::Debug + Send + Sync + 'static -{ +pub trait MetricReader: TemporalitySelector + fmt::Debug + Send + Sync + 'static { /// Registers a [MetricReader] with a [Pipeline]. /// /// The pipeline argument allows the `MetricReader` to signal the sdk to collect @@ -95,65 +92,3 @@ impl TemporalitySelector for DefaultTemporalitySelector { Temporality::Cumulative } } - -/// An interface for selecting the aggregation and the parameters for an -/// [InstrumentKind]. -pub trait AggregationSelector: Send + Sync { - /// Selects the aggregation and the parameters to use for that aggregation based on - /// the [InstrumentKind]. - fn aggregation(&self, kind: InstrumentKind) -> Aggregation; -} - -impl<T> AggregationSelector for T -where - T: Fn(InstrumentKind) -> Aggregation + Send + Sync, -{ - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - self(kind) - } -} - -/// The default aggregation and parameters for an instrument of [InstrumentKind]. -/// -/// This [AggregationSelector] uses the following selection mapping per [the spec]: -/// -/// * Counter ⇨ Sum -/// * Observable Counter ⇨ Sum -/// * UpDownCounter ⇨ Sum -/// * Observable UpDownCounter ⇨ Sum -/// * Gauge ⇨ LastValue -/// * Observable Gauge ⇨ LastValue -/// * Histogram ⇨ ExplicitBucketHistogram -/// -/// [the spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.19.0/specification/metrics/sdk.md#default-aggregation -#[derive(Clone, Default, Debug)] -pub struct DefaultAggregationSelector { - pub(crate) _private: (), -} - -impl DefaultAggregationSelector { - /// Create a new default aggregation selector. - pub fn new() -> Self { - Self::default() - } -} - -impl AggregationSelector for DefaultAggregationSelector { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - match kind { - InstrumentKind::Counter - | InstrumentKind::UpDownCounter - | InstrumentKind::ObservableCounter - | InstrumentKind::ObservableUpDownCounter => Aggregation::Sum, - InstrumentKind::Gauge => Aggregation::LastValue, - InstrumentKind::ObservableGauge => Aggregation::LastValue, - InstrumentKind::Histogram => Aggregation::ExplicitBucketHistogram { - boundaries: vec![ - 0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 750.0, 1000.0, 2500.0, - 5000.0, 7500.0, 10000.0, - ], - record_min_max: true, - }, - } - } -} diff --git a/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs b/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs index d28cd4062f..3f85b360b7 100644 --- a/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs +++ b/opentelemetry-sdk/src/testing/metrics/in_memory_exporter.rs @@ -1,10 +1,7 @@ use crate::metrics::data::{Histogram, Metric, ResourceMetrics, ScopeMetrics, Temporality}; use crate::metrics::exporter::PushMetricsExporter; -use crate::metrics::reader::{ - AggregationSelector, DefaultAggregationSelector, DefaultTemporalitySelector, - TemporalitySelector, -}; -use crate::metrics::{data, Aggregation, InstrumentKind}; +use crate::metrics::reader::{DefaultTemporalitySelector, TemporalitySelector}; +use crate::metrics::{data, InstrumentKind}; use async_trait::async_trait; use opentelemetry::metrics::MetricsError; use opentelemetry::metrics::Result; @@ -61,7 +58,6 @@ use std::sync::{Arc, Mutex}; /// ``` pub struct InMemoryMetricsExporter { metrics: Arc<Mutex<VecDeque<ResourceMetrics>>>, - aggregation_selector: Arc<dyn AggregationSelector + Send + Sync>, temporality_selector: Arc<dyn TemporalitySelector + Send + Sync>, } @@ -69,7 +65,6 @@ impl Clone for InMemoryMetricsExporter { fn clone(&self) -> Self { InMemoryMetricsExporter { metrics: self.metrics.clone(), - aggregation_selector: self.aggregation_selector.clone(), temporality_selector: self.temporality_selector.clone(), } } @@ -96,7 +91,6 @@ impl Default for InMemoryMetricsExporter { /// let exporter = InMemoryMetricsExporterBuilder::new().build(); /// ``` pub struct InMemoryMetricsExporterBuilder { - aggregation_selector: Option<Arc<dyn AggregationSelector + Send + Sync>>, temporality_selector: Option<Arc<dyn TemporalitySelector + Send + Sync>>, } @@ -116,20 +110,10 @@ impl InMemoryMetricsExporterBuilder { /// Creates a new instance of the `InMemoryMetricsExporterBuilder`. pub fn new() -> Self { Self { - aggregation_selector: None, temporality_selector: None, } } - /// Sets the aggregation selector for the exporter. - pub fn with_aggregation_selector<T>(mut self, aggregation_selector: T) -> Self - where - T: AggregationSelector + Send + Sync + 'static, - { - self.aggregation_selector = Some(Arc::new(aggregation_selector)); - self - } - /// Sets the temporality selector for the exporter. pub fn with_temporality_selector<T>(mut self, temporality_selector: T) -> Self where @@ -144,9 +128,6 @@ impl InMemoryMetricsExporterBuilder { pub fn build(self) -> InMemoryMetricsExporter { InMemoryMetricsExporter { metrics: Arc::new(Mutex::new(VecDeque::new())), - aggregation_selector: self - .aggregation_selector - .unwrap_or_else(|| Arc::new(DefaultAggregationSelector::default())), temporality_selector: self .temporality_selector .unwrap_or_else(|| Arc::new(DefaultTemporalitySelector::default())), @@ -270,12 +251,6 @@ impl InMemoryMetricsExporter { } } -impl AggregationSelector for InMemoryMetricsExporter { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - self.aggregation_selector.aggregation(kind) - } -} - impl TemporalitySelector for InMemoryMetricsExporter { fn temporality(&self, kind: InstrumentKind) -> Temporality { self.temporality_selector.temporality(kind) diff --git a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs index 6bea3fdd02..2056758a41 100644 --- a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs +++ b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs @@ -1,11 +1,10 @@ use std::sync::{Arc, Mutex, Weak}; use crate::metrics::{ - aggregation::Aggregation, data::{ResourceMetrics, Temporality}, instrument::InstrumentKind, pipeline::Pipeline, - reader::{AggregationSelector, MetricReader, TemporalitySelector}, + reader::{MetricReader, TemporalitySelector}, }; use opentelemetry::metrics::Result; @@ -55,12 +54,6 @@ impl MetricReader for TestMetricReader { } } -impl AggregationSelector for TestMetricReader { - fn aggregation(&self, _kind: InstrumentKind) -> Aggregation { - Aggregation::Drop - } -} - impl TemporalitySelector for TestMetricReader { fn temporality(&self, _kind: InstrumentKind) -> Temporality { Temporality::Cumulative diff --git a/opentelemetry-stdout/src/metrics/exporter.rs b/opentelemetry-stdout/src/metrics/exporter.rs index a961222d46..fd39f8919e 100644 --- a/opentelemetry-stdout/src/metrics/exporter.rs +++ b/opentelemetry-stdout/src/metrics/exporter.rs @@ -5,11 +5,8 @@ use opentelemetry::metrics::{MetricsError, Result}; use opentelemetry_sdk::metrics::{ data::{self, ScopeMetrics}, exporter::PushMetricsExporter, - reader::{ - AggregationSelector, DefaultAggregationSelector, DefaultTemporalitySelector, - TemporalitySelector, - }, - Aggregation, InstrumentKind, + reader::{DefaultTemporalitySelector, TemporalitySelector}, + InstrumentKind, }; use std::fmt::Debug; use std::sync::atomic; @@ -18,7 +15,6 @@ use std::sync::atomic; pub struct MetricsExporter { is_shutdown: atomic::AtomicBool, temporality_selector: Box<dyn TemporalitySelector>, - aggregation_selector: Box<dyn AggregationSelector>, } impl MetricsExporter { @@ -45,12 +41,6 @@ impl TemporalitySelector for MetricsExporter { } } -impl AggregationSelector for MetricsExporter { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - self.aggregation_selector.aggregation(kind) - } -} - #[async_trait] impl PushMetricsExporter for MetricsExporter { /// Write Metrics to stdout @@ -234,7 +224,6 @@ fn print_hist_data_points<T: Debug>(data_points: &[data::HistogramDataPoint<T>]) #[derive(Default)] pub struct MetricsExporterBuilder { temporality_selector: Option<Box<dyn TemporalitySelector>>, - aggregation_selector: Option<Box<dyn AggregationSelector>>, } impl MetricsExporterBuilder { @@ -247,24 +236,12 @@ impl MetricsExporterBuilder { self } - /// Set the aggregation exporter for the exporter - pub fn with_aggregation_selector( - mut self, - selector: impl AggregationSelector + 'static, - ) -> Self { - self.aggregation_selector = Some(Box::new(selector)); - self - } - /// Create a metrics exporter with the current configuration pub fn build(self) -> MetricsExporter { MetricsExporter { temporality_selector: self .temporality_selector .unwrap_or_else(|| Box::new(DefaultTemporalitySelector::new())), - aggregation_selector: self - .aggregation_selector - .unwrap_or_else(|| Box::new(DefaultAggregationSelector::new())), is_shutdown: atomic::AtomicBool::new(false), } }