Address review comments #1

Author: fraillt

opentelemetry-sdk/src/metrics/data/mod.rs

@@ -54,11 +54,11 @@ pub trait Aggregation: fmt::Debug + any::Any + Send + Sync {
 }
 
 /// Allow to access data points of an [Aggregation].
-pub trait AggregationDataPoints {
+pub(crate) trait AggregationDataPoints {
     /// The type of data point in the aggregation.
-    type Point;
+    type DataPoint;
     /// The data points of the aggregation.
-    fn points(&mut self) -> &mut Vec<Self::Point>;
+    fn points(&mut self) -> &mut Vec<Self::DataPoint>;
 }
 
 /// DataPoint is a single data point in a time series.
@@ -237,9 +237,9 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for ExponentialHistogram
 }
 
 impl<T> AggregationDataPoints for ExponentialHistogram<T> {
-    type Point = ExponentialHistogramDataPoint<T>;
+    type DataPoint = ExponentialHistogramDataPoint<T>;
 
-    fn points(&mut self) -> &mut Vec<Self::Point> {
+    fn points(&mut self) -> &mut Vec<Self::DataPoint> {
         &mut self.data_points
     }
 }

opentelemetry-sdk/src/metrics/internal/aggregate.rs

@@ -198,29 +198,29 @@ impl<T: Number> AggregateBuilder<T> {
         record_sum: bool,
     ) -> AggregateFns<T> {
         match self.temporality {
-            Temporality::Delta => ExpoHistogram {
-                aggregate_collector: Collector::new(
+            Temporality::Delta => ExpoHistogram::new(
+                Collector::new(
                     self.filter.clone(),
                     DeltaValueMap::new(ExpoHistogramBucketConfig {
                         max_size: max_size as i32,
                         max_scale,
                     }),
                 ),
-                record_min_max,
                 record_sum,
-            }
+                record_min_max,
+            )
             .into(),
-            _ => ExpoHistogram {
-                aggregate_collector: Collector::new(
+            _ => ExpoHistogram::new(
+                Collector::new(
                     self.filter.clone(),
                     CumulativeValueMap::new(ExpoHistogramBucketConfig {
                         max_size: max_size as i32,
                         max_scale,
                     }),
                 ),
-                record_min_max,
                 record_sum,
-            }
+                record_min_max,
+            )
             .into(),
         }
     }

opentelemetry-sdk/src/metrics/internal/collector.rs

@@ -40,7 +40,7 @@ pub(crate) trait AggregateCollector: Send + Sync + 'static {
         F: FnMut(
             Vec<KeyValue>,
             &Self::Aggr,
-        ) -> <InitAggregate::Aggr as AggregationDataPoints>::Point;
+        ) -> <InitAggregate::Aggr as AggregationDataPoints>::DataPoint;
 }
 
 pub(crate) struct Collector<AM> {
@@ -92,7 +92,10 @@ where
     ) -> (usize, Option<Box<dyn Aggregation>>)
     where
         InitAggregate: InitAggregationData,
-        F: FnMut(Vec<KeyValue>, &AM::Aggr) -> <InitAggregate::Aggr as AggregationDataPoints>::Point,
+        F: FnMut(
+            Vec<KeyValue>,
+            &AM::Aggr,
+        ) -> <InitAggregate::Aggr as AggregationDataPoints>::DataPoint,
     {
         let time = self.init_time();
         let s_data = dest.and_then(|d| d.as_mut().downcast_mut::<InitAggregate::Aggr>());

opentelemetry-sdk/src/metrics/internal/exponential_histogram.rs

@@ -349,9 +349,19 @@ pub(crate) struct ExpoHistogramBucketConfig {
 /// Each histogram is scoped by attributes and the aggregation cycle the
 /// measurements were made in.
 pub(crate) struct ExpoHistogram<AC> {
-    pub(crate) aggregate_collector: AC,
-    pub(crate) record_sum: bool,
-    pub(crate) record_min_max: bool,
+    aggregate_collector: AC,
+    record_sum: bool,
+    record_min_max: bool,
+}
+
+impl<AC> ExpoHistogram<AC> {
+    pub(crate) fn new(aggregate_collector: AC, record_sum: bool, record_min_max: bool) -> Self {
+        Self {
+            aggregate_collector,
+            record_sum,
+            record_min_max,
+        }
+    }
 }
 
 impl<AC, T> Measure<T> for ExpoHistogram<AC>