Skip to content

Commit

Permalink
Address review comments #1
Browse files Browse the repository at this point in the history
  • Loading branch information
fraillt committed Jan 21, 2025
1 parent ec61467 commit ea151e4
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 18 deletions.
10 changes: 5 additions & 5 deletions opentelemetry-sdk/src/metrics/data/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
}
Expand Down
16 changes: 8 additions & 8 deletions opentelemetry-sdk/src/metrics/internal/aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
Expand Down
7 changes: 5 additions & 2 deletions opentelemetry-sdk/src/metrics/internal/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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>());
Expand Down
16 changes: 13 additions & 3 deletions opentelemetry-sdk/src/metrics/internal/exponential_histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit ea151e4

Please sign in to comment.