Skip to content

Commit e4cba94

Browse files
frailltfraillt
authored
Move time from DataPoint to Histogram/ExpoHistogram (#2411)
1 parent 1a4e931 commit e4cba94

File tree

7 files changed

+85
-82
lines changed

7 files changed

+85
-82
lines changed

‎opentelemetry-proto/src/transform/metrics.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ pub mod tonic {
230230
.iter()
231231
.map(|dp| TonicHistogramDataPoint {
232232
attributes: dp.attributes.iter().map(Into::into).collect(),
233-
start_time_unix_nano: to_nanos(dp.start_time),
234-
time_unix_nano: to_nanos(dp.time),
233+
start_time_unix_nano: to_nanos(hist.start_time),
234+
time_unix_nano: to_nanos(hist.time),
235235
count: dp.count,
236236
sum: Some(dp.sum.into_f64()),
237237
bucket_counts: dp.bucket_counts.clone(),
@@ -258,8 +258,8 @@ pub mod tonic {
258258
.iter()
259259
.map(|dp| TonicExponentialHistogramDataPoint {
260260
attributes: dp.attributes.iter().map(Into::into).collect(),
261-
start_time_unix_nano: to_nanos(dp.start_time),
262-
time_unix_nano: to_nanos(dp.time),
261+
start_time_unix_nano: to_nanos(hist.start_time),
262+
time_unix_nano: to_nanos(hist.time),
263263
count: dp.count as u64,
264264
sum: Some(dp.sum.into_f64()),
265265
scale: dp.scale.into(),

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

Lines changed: 8 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for Sum<T> {
147147
pub struct Histogram<T> {
148148
/// Individual aggregated measurements with unique attributes.
149149
pub data_points: Vec<HistogramDataPoint<T>>,
150+
/// The time when the time series was started.
151+
pub start_time: SystemTime,
152+
/// The time when the time series was recorded.
153+
pub time: SystemTime,
150154
/// Describes if the aggregation is reported as the change from the last report
151155
/// time, or the cumulative changes since a fixed start time.
152156
pub temporality: Temporality,
@@ -166,11 +170,6 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for Histogram<T> {
166170
pub struct HistogramDataPoint<T> {
167171
/// The set of key value pairs that uniquely identify the time series.
168172
pub attributes: Vec<KeyValue>,
169-
/// The time when the time series was started.
170-
pub start_time: SystemTime,
171-
/// The time when the time series was recorded.
172-
pub time: SystemTime,
173-
174173
/// The number of updates this histogram has been calculated with.
175174
pub count: u64,
176175
/// The upper bounds of the buckets of the histogram.
@@ -195,8 +194,6 @@ impl<T: Copy> Clone for HistogramDataPoint<T> {
195194
fn clone(&self) -> Self {
196195
Self {
197196
attributes: self.attributes.clone(),
198-
start_time: self.start_time,
199-
time: self.time,
200197
count: self.count,
201198
bounds: self.bounds.clone(),
202199
bucket_counts: self.bucket_counts.clone(),
@@ -213,7 +210,10 @@ impl<T: Copy> Clone for HistogramDataPoint<T> {
213210
pub struct ExponentialHistogram<T> {
214211
/// The individual aggregated measurements with unique attributes.
215212
pub data_points: Vec<ExponentialHistogramDataPoint<T>>,
216-
213+
/// When the time series was started.
214+
pub start_time: SystemTime,
215+
/// The time when the time series was recorded.
216+
pub time: SystemTime,
217217
/// Describes if the aggregation is reported as the change from the last report
218218
/// time, or the cumulative changes since a fixed start time.
219219
pub temporality: Temporality,
@@ -233,10 +233,6 @@ impl<T: fmt::Debug + Send + Sync + 'static> Aggregation for ExponentialHistogram
233233
pub struct ExponentialHistogramDataPoint<T> {
234234
/// The set of key value pairs that uniquely identify the time series.
235235
pub attributes: Vec<KeyValue>,
236-
/// When the time series was started.
237-
pub start_time: SystemTime,
238-
/// The time when the time series was recorded.
239-
pub time: SystemTime,
240236

241237
/// The number of updates this histogram has been calculated with.
242238
pub count: usize,
@@ -281,8 +277,6 @@ impl<T: Copy> Clone for ExponentialHistogramDataPoint<T> {
281277
fn clone(&self) -> Self {
282278
Self {
283279
attributes: self.attributes.clone(),
284-
start_time: self.start_time,
285-
time: self.time,
286280
count: self.count,
287281
min: self.min,
288282
max: self.max,
@@ -375,8 +369,6 @@ mod tests {
375369

376370
let histogram_data_point = HistogramDataPoint {
377371
attributes: vec![KeyValue::new("key", "value")],
378-
start_time: std::time::SystemTime::now(),
379-
time: std::time::SystemTime::now(),
380372
count: 0,
381373
bounds: vec![],
382374
bucket_counts: vec![],
@@ -395,8 +387,6 @@ mod tests {
395387

396388
let exponential_histogram_data_point = ExponentialHistogramDataPoint {
397389
attributes: vec![KeyValue::new("key", "value")],
398-
start_time: std::time::SystemTime::now(),
399-
time: std::time::SystemTime::now(),
400390
count: 0,
401391
min: None,
402392
max: None,

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,6 @@ mod tests {
314314
let mut a = Histogram {
315315
data_points: vec![HistogramDataPoint {
316316
attributes: vec![KeyValue::new("a1", 1)],
317-
start_time: SystemTime::now(),
318-
time: SystemTime::now(),
319317
count: 2,
320318
bounds: vec![1.0, 2.0],
321319
bucket_counts: vec![0, 1, 1],
@@ -324,6 +322,8 @@ mod tests {
324322
sum: 3u64,
325323
exemplars: vec![],
326324
}],
325+
start_time: SystemTime::now(),
326+
time: SystemTime::now(),
327327
temporality: if temporality == Temporality::Delta {
328328
Temporality::Cumulative
329329
} else {
@@ -357,8 +357,6 @@ mod tests {
357357
let mut a = ExponentialHistogram {
358358
data_points: vec![ExponentialHistogramDataPoint {
359359
attributes: vec![KeyValue::new("a1", 1)],
360-
start_time: SystemTime::now(),
361-
time: SystemTime::now(),
362360
count: 2,
363361
min: None,
364362
max: None,
@@ -376,6 +374,8 @@ mod tests {
376374
zero_threshold: 1.0,
377375
exemplars: vec![],
378376
}],
377+
start_time: SystemTime::now(),
378+
time: SystemTime::now(),
379379
temporality: if temporality == Temporality::Delta {
380380
Temporality::Cumulative
381381
} else {

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

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -389,33 +389,34 @@ impl<T: Number> ExpoHistogram<T> {
389389
&self,
390390
dest: Option<&mut dyn Aggregation>,
391391
) -> (usize, Option<Box<dyn Aggregation>>) {
392-
let t = SystemTime::now();
392+
let time = SystemTime::now();
393+
let start_time = self
394+
.start
395+
.lock()
396+
.map(|mut start| replace(start.deref_mut(), time))
397+
.unwrap_or(time);
393398

394399
let h = dest.and_then(|d| d.as_mut().downcast_mut::<data::ExponentialHistogram<T>>());
395400
let mut new_agg = if h.is_none() {
396401
Some(data::ExponentialHistogram {
397402
data_points: vec![],
403+
start_time,
404+
time,
398405
temporality: Temporality::Delta,
399406
})
400407
} else {
401408
None
402409
};
403410
let h = h.unwrap_or_else(|| new_agg.as_mut().expect("present if h is none"));
404411
h.temporality = Temporality::Delta;
405-
406-
let prev_start = self
407-
.start
408-
.lock()
409-
.map(|mut start| replace(start.deref_mut(), t))
410-
.unwrap_or(t);
412+
h.start_time = start_time;
413+
h.time = time;
411414

412415
self.value_map
413416
.collect_and_reset(&mut h.data_points, |attributes, attr| {
414417
let b = attr.into_inner().unwrap_or_else(|err| err.into_inner());
415418
data::ExponentialHistogramDataPoint {
416419
attributes,
417-
start_time: prev_start,
418-
time: t,
419420
count: b.count,
420421
min: if self.record_min_max {
421422
Some(b.min)
@@ -450,33 +451,34 @@ impl<T: Number> ExpoHistogram<T> {
450451
&self,
451452
dest: Option<&mut dyn Aggregation>,
452453
) -> (usize, Option<Box<dyn Aggregation>>) {
453-
let t = SystemTime::now();
454+
let time = SystemTime::now();
455+
let start_time = self
456+
.start
457+
.lock()
458+
.map(|s| *s)
459+
.unwrap_or_else(|_| SystemTime::now());
454460

455461
let h = dest.and_then(|d| d.as_mut().downcast_mut::<data::ExponentialHistogram<T>>());
456462
let mut new_agg = if h.is_none() {
457463
Some(data::ExponentialHistogram {
458464
data_points: vec![],
465+
start_time,
466+
time,
459467
temporality: Temporality::Cumulative,
460468
})
461469
} else {
462470
None
463471
};
464472
let h = h.unwrap_or_else(|| new_agg.as_mut().expect("present if h is none"));
465473
h.temporality = Temporality::Cumulative;
466-
467-
let prev_start = self
468-
.start
469-
.lock()
470-
.map(|s| *s)
471-
.unwrap_or_else(|_| SystemTime::now());
474+
h.start_time = start_time;
475+
h.time = time;
472476

473477
self.value_map
474478
.collect_readonly(&mut h.data_points, |attributes, attr| {
475479
let b = attr.lock().unwrap_or_else(|err| err.into_inner());
476480
data::ExponentialHistogramDataPoint {
477481
attributes,
478-
start_time: prev_start,
479-
time: t,
480482
count: b.count,
481483
min: if self.record_min_max {
482484
Some(b.min)
@@ -1270,8 +1272,6 @@ mod tests {
12701272
min: Some(1.into()),
12711273
max: Some(16.into()),
12721274
sum: 31.into(),
1273-
start_time: SystemTime::now(),
1274-
time: SystemTime::now(),
12751275
scale: -1,
12761276
positive_bucket: data::ExponentialBucket {
12771277
offset: -1,
@@ -1285,6 +1285,8 @@ mod tests {
12851285
zero_threshold: 0.0,
12861286
zero_count: 0,
12871287
}],
1288+
start_time: SystemTime::now(),
1289+
time: SystemTime::now(),
12881290
},
12891291
want_count: 1,
12901292
},
@@ -1318,8 +1320,6 @@ mod tests {
13181320
offset: -1,
13191321
counts: vec![1, 4, 1],
13201322
},
1321-
start_time: SystemTime::now(),
1322-
time: SystemTime::now(),
13231323
negative_bucket: data::ExponentialBucket {
13241324
offset: 0,
13251325
counts: vec![],
@@ -1328,6 +1328,8 @@ mod tests {
13281328
zero_threshold: 0.0,
13291329
zero_count: 0,
13301330
}],
1331+
start_time: SystemTime::now(),
1332+
time: SystemTime::now(),
13311333
},
13321334
want_count: 1,
13331335
},
@@ -1364,8 +1366,6 @@ mod tests {
13641366
offset: -1,
13651367
counts: vec![1, 4, 1],
13661368
},
1367-
start_time: SystemTime::now(),
1368-
time: SystemTime::now(),
13691369
negative_bucket: data::ExponentialBucket {
13701370
offset: 0,
13711371
counts: vec![],
@@ -1374,6 +1374,8 @@ mod tests {
13741374
zero_threshold: 0.0,
13751375
zero_count: 0,
13761376
}],
1377+
start_time: SystemTime::now(),
1378+
time: SystemTime::now(),
13771379
},
13781380
want_count: 1,
13791381
},
@@ -1410,8 +1412,6 @@ mod tests {
14101412
counts: vec![1, 6, 2],
14111413
},
14121414
attributes: vec![],
1413-
start_time: SystemTime::now(),
1414-
time: SystemTime::now(),
14151415
negative_bucket: data::ExponentialBucket {
14161416
offset: 0,
14171417
counts: vec![],
@@ -1420,6 +1420,8 @@ mod tests {
14201420
zero_threshold: 0.0,
14211421
zero_count: 0,
14221422
}],
1423+
start_time: SystemTime::now(),
1424+
time: SystemTime::now(),
14231425
},
14241426
want_count: 1,
14251427
},
@@ -1430,6 +1432,8 @@ mod tests {
14301432

14311433
let mut got: Box<dyn data::Aggregation> = Box::new(data::ExponentialHistogram::<T> {
14321434
data_points: vec![],
1435+
start_time: SystemTime::now(),
1436+
time: SystemTime::now(),
14331437
temporality: Temporality::Delta,
14341438
});
14351439
let mut count = 0;

‎opentelemetry-sdk/src/metrics/internal/histogram.rs‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -109,32 +109,33 @@ impl<T: Number> Histogram<T> {
109109
&self,
110110
dest: Option<&mut dyn Aggregation>,
111111
) -> (usize, Option<Box<dyn Aggregation>>) {
112-
let t = SystemTime::now();
112+
let time = SystemTime::now();
113+
let start_time = self
114+
.start
115+
.lock()
116+
.map(|mut start| replace(start.deref_mut(), time))
117+
.unwrap_or(time);
113118
let h = dest.and_then(|d| d.as_mut().downcast_mut::<data::Histogram<T>>());
114119
let mut new_agg = if h.is_none() {
115120
Some(data::Histogram {
116121
data_points: vec![],
122+
start_time,
123+
time,
117124
temporality: Temporality::Delta,
118125
})
119126
} else {
120127
None
121128
};
122129
let h = h.unwrap_or_else(|| new_agg.as_mut().expect("present if h is none"));
123130
h.temporality = Temporality::Delta;
124-
125-
let prev_start = self
126-
.start
127-
.lock()
128-
.map(|mut start| replace(start.deref_mut(), t))
129-
.unwrap_or(t);
131+
h.start_time = start_time;
132+
h.time = time;
130133

131134
self.value_map
132135
.collect_and_reset(&mut h.data_points, |attributes, aggr| {
133136
let b = aggr.into_inner().unwrap_or_else(|err| err.into_inner());
134137
HistogramDataPoint {
135138
attributes,
136-
start_time: prev_start,
137-
time: t,
138139
count: b.count,
139140
bounds: self.bounds.clone(),
140141
bucket_counts: b.counts,
@@ -164,32 +165,34 @@ impl<T: Number> Histogram<T> {
164165
&self,
165166
dest: Option<&mut dyn Aggregation>,
166167
) -> (usize, Option<Box<dyn Aggregation>>) {
167-
let t = SystemTime::now();
168+
let time = SystemTime::now();
169+
let start_time = self
170+
.start
171+
.lock()
172+
.map(|s| *s)
173+
.unwrap_or_else(|_| SystemTime::now());
174+
168175
let h = dest.and_then(|d| d.as_mut().downcast_mut::<data::Histogram<T>>());
169176
let mut new_agg = if h.is_none() {
170177
Some(data::Histogram {
171178
data_points: vec![],
179+
start_time,
180+
time,
172181
temporality: Temporality::Cumulative,
173182
})
174183
} else {
175184
None
176185
};
177186
let h = h.unwrap_or_else(|| new_agg.as_mut().expect("present if h is none"));
178187
h.temporality = Temporality::Cumulative;
179-
180-
let prev_start = self
181-
.start
182-
.lock()
183-
.map(|s| *s)
184-
.unwrap_or_else(|_| SystemTime::now());
188+
h.start_time = start_time;
189+
h.time = time;
185190

186191
self.value_map
187192
.collect_readonly(&mut h.data_points, |attributes, aggr| {
188193
let b = aggr.lock().unwrap_or_else(|err| err.into_inner());
189194
HistogramDataPoint {
190195
attributes,
191-
start_time: prev_start,
192-
time: t,
193196
count: b.count,
194197
bounds: self.bounds.clone(),
195198
bucket_counts: b.counts.clone(),

0 commit comments

Comments
 (0)