AggregationSelector is not needed anymore (#2085)

Author: fraillt

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))
     }
 }
 

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

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();
 //!     # }

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,
         }
     }
 }

opentelemetry-otlp/tests/integration_test/expected/serialized_traces.json

@@ -112,6 +112,12 @@
                       "value": {
                         "intValue": "100"
                       }
+                    },
+                    {
+                      "key": "number/int",
+                      "value": {
+                        "intValue": "100"
+                      }
                     }
                   ],
                   "droppedAttributesCount": 0

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)

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

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

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)
     }
 }