-
Notifications
You must be signed in to change notification settings - Fork 472
Synchronous Metric collection (Delta , Cumulative) #1265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 33 commits
c08cf38
c729c6a
f692e15
ffb54ee
8ecf729
18e4f27
ad471d0
9cc53a4
05c1a8d
f0380a9
fc657f9
e702e89
d12686e
884f636
1874375
12ba367
e011a2b
5ab4f75
c30b234
618de73
66d7af0
e49e7c4
da5eb93
a102d7f
0551aeb
b5d3bd5
b4bac61
015757c
ae65b1d
29ee87d
4b31fd3
62d0e92
464abf7
4c26336
cb1c5a4
0a62679
6919d1d
cf3c272
010d1bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,24 +11,42 @@ namespace sdk | |
{ | ||
namespace metrics | ||
{ | ||
class InstrumentMonotonicityAwareAggregation | ||
{ | ||
public: | ||
InstrumentMonotonicityAwareAggregation(bool is_monotonic) : is_monotonic_(is_monotonic) {} | ||
bool IsMonotonic() { return is_monotonic_; } | ||
|
||
private: | ||
bool is_monotonic_; | ||
}; | ||
|
||
class Aggregation | ||
{ | ||
public: | ||
virtual void Aggregate(long value, const PointAttributes &attributes = {}) noexcept = 0; | ||
|
||
virtual void Aggregate(double value, const PointAttributes &attributes = {}) noexcept = 0; | ||
|
||
virtual PointType Collect() noexcept = 0; | ||
/** | ||
* Returns the result of the merge of the two aggregations. | ||
* | ||
* This should always assume that the aggregations do not overlap and merge together for a new | ||
* cumulative report. | ||
* | ||
* @param delta the newly captured (delta) aggregation | ||
* @return the result of the merge of the given aggregation. | ||
*/ | ||
|
||
virtual std::unique_ptr<Aggregation> Merge(Aggregation &delta) noexcept = 0; | ||
|
||
/** | ||
* Returns a new DELTA aggregation by comparing two cumulative measurements. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is all upper case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not intended, made it all small case now :) |
||
* | ||
* @param next the newly captured (cumulative) aggregation. | ||
* @return The resulting delta aggregation. | ||
*/ | ||
virtual std::unique_ptr<Aggregation> Diff(Aggregation &next) noexcept = 0; | ||
|
||
/** | ||
* Returns the point data that the aggregation will produce. | ||
* | ||
* @return PointType | ||
*/ | ||
|
||
virtual PointType ToPoint() noexcept = 0; | ||
|
||
virtual ~Aggregation() = default; | ||
}; | ||
|
||
} // namespace metrics | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,57 +13,47 @@ namespace sdk | |
{ | ||
namespace metrics | ||
{ | ||
template <class T> | ||
static inline void PopulateHistogramDataPoint(HistogramPointData &histogram, | ||
opentelemetry::common::SystemTimestamp epoch_nanos, | ||
T sum, | ||
uint64_t count, | ||
std::vector<uint64_t> &counts, | ||
std::vector<T> boundaries) | ||
{ | ||
histogram.epoch_nanos_ = epoch_nanos; | ||
histogram.boundaries_ = boundaries; | ||
histogram.sum_ = sum; | ||
histogram.counts_ = counts; | ||
histogram.count_ = count; | ||
} | ||
|
||
class LongHistogramAggregation : public Aggregation | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this be a template class as it almost the same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was discussed in one of the earlier PR here: |
||
{ | ||
public: | ||
LongHistogramAggregation(); | ||
LongHistogramAggregation(HistogramPointData &&); | ||
|
||
void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override; | ||
|
||
void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override {} | ||
|
||
PointType Collect() noexcept override; | ||
virtual std::unique_ptr<Aggregation> Merge(Aggregation &delta) noexcept override; | ||
|
||
virtual std::unique_ptr<Aggregation> Diff(Aggregation &next) noexcept override; | ||
|
||
PointType ToPoint() noexcept override; | ||
|
||
private: | ||
opentelemetry::common::SpinLockMutex lock_; | ||
std::vector<long> boundaries_; | ||
long sum_; | ||
std::vector<uint64_t> counts_; | ||
uint64_t count_; | ||
HistogramPointData point_data_; | ||
}; | ||
|
||
class DoubleHistogramAggregation : public Aggregation | ||
{ | ||
public: | ||
DoubleHistogramAggregation(); | ||
DoubleHistogramAggregation(HistogramPointData &&); | ||
|
||
void Aggregate(long value, const PointAttributes &attributes = {}) noexcept override {} | ||
|
||
void Aggregate(double value, const PointAttributes &attributes = {}) noexcept override; | ||
|
||
PointType Collect() noexcept override; | ||
virtual std::unique_ptr<Aggregation> Merge(Aggregation &delta) noexcept override; | ||
|
||
virtual std::unique_ptr<Aggregation> Diff(Aggregation &next) noexcept override; | ||
|
||
PointType ToPoint() noexcept override; | ||
|
||
private: | ||
opentelemetry::common::SpinLockMutex lock_; | ||
std::vector<double> boundaries_; | ||
double sum_; | ||
std::vector<uint64_t> counts_; | ||
uint64_t count_; | ||
HistogramPointData point_data_; | ||
}; | ||
|
||
} // namespace metrics | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the new virtual function be marked as
const
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, have marked Merge, Diff and ToPoint methods as
const