Skip to content

Commit 20ba935

Browse files
ethandmdethandmd
committed
refactor unique_ptr for buckets with deep copies
1 parent 5641bbd commit 20ba935

File tree

7 files changed

+243
-112
lines changed

7 files changed

+243
-112
lines changed

‎exporters/ostream/src/metric_exporter.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
252252
{
253253
auto histogram_point_data =
254254
nostd::get<sdk::metrics::Base2ExponentialHistogramPointData>(point_data);
255+
if (!histogram_point_data.positive_buckets_ && !histogram_point_data.negative_buckets_)
256+
{
257+
return;
258+
}
255259
sout_ << "\n type: Base2ExponentialHistogramPointData";
256260
sout_ << "\n count: " << histogram_point_data.count_;
257261
sout_ << "\n sum: " << histogram_point_data.sum_;
@@ -263,21 +267,21 @@ void OStreamMetricExporter::printPointData(const opentelemetry::sdk::metrics::Po
263267
}
264268
sout_ << "\n scale: " << histogram_point_data.scale_;
265269
sout_ << "\n positive buckets:";
266-
if (!histogram_point_data.positive_buckets_.Empty())
270+
if (!histogram_point_data.positive_buckets_->Empty())
267271
{
268-
for (auto i = histogram_point_data.positive_buckets_.StartIndex();
269-
i <= histogram_point_data.positive_buckets_.EndIndex(); ++i)
272+
for (auto i = histogram_point_data.positive_buckets_->StartIndex();
273+
i <= histogram_point_data.positive_buckets_->EndIndex(); ++i)
270274
{
271-
sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_.Get(i);
275+
sout_ << "\n\t" << i << ": " << histogram_point_data.positive_buckets_->Get(i);
272276
}
273277
}
274278
sout_ << "\n negative buckets:";
275-
if (!histogram_point_data.negative_buckets_.Empty())
279+
if (!histogram_point_data.negative_buckets_->Empty())
276280
{
277-
for (auto i = histogram_point_data.negative_buckets_.StartIndex();
278-
i <= histogram_point_data.negative_buckets_.EndIndex(); ++i)
281+
for (auto i = histogram_point_data.negative_buckets_->StartIndex();
282+
i <= histogram_point_data.negative_buckets_->EndIndex(); ++i)
279283
{
280-
sout_ << "\n\t" << i << ": " << histogram_point_data.negative_buckets_.Get(i);
284+
sout_ << "\n\t" << i << ": " << histogram_point_data.negative_buckets_->Get(i);
281285
}
282286
}
283287
}

‎exporters/ostream/test/ostream_metric_test.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,11 @@ TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData)
194194
histogram_point_data1.record_min_max_ = true;
195195
histogram_point_data1.zero_count_ = 1;
196196
histogram_point_data1.positive_buckets_ =
197-
opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10);
197+
std::make_unique<opentelemetry::sdk::metrics::AdaptingCircularBufferCounter>(10);
198198
histogram_point_data1.negative_buckets_ =
199-
opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10);
200-
histogram_point_data1.positive_buckets_.Increment(1, 1);
201-
histogram_point_data1.negative_buckets_.Increment(-2, 1);
199+
std::make_unique<opentelemetry::sdk::metrics::AdaptingCircularBufferCounter>(10);
200+
histogram_point_data1.positive_buckets_->Increment(1, 1);
201+
histogram_point_data1.negative_buckets_->Increment(-2, 1);
202202

203203
metric_sdk::Base2ExponentialHistogramPointData histogram_point_data2;
204204
histogram_point_data2.count_ = 4;
@@ -209,12 +209,12 @@ TEST(OStreamMetricsExporter, ExportBase2ExponentialHistogramPointData)
209209
histogram_point_data2.record_min_max_ = false;
210210
histogram_point_data2.zero_count_ = 2;
211211
histogram_point_data2.positive_buckets_ =
212-
opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10);
212+
std::make_unique<opentelemetry::sdk::metrics::AdaptingCircularBufferCounter>(10);
213213
histogram_point_data2.negative_buckets_ =
214-
opentelemetry::sdk::metrics::AdaptingCircularBufferCounter(10);
215-
histogram_point_data2.positive_buckets_.Increment(3, 1);
216-
histogram_point_data2.negative_buckets_.Increment(-2, 1);
217-
histogram_point_data2.negative_buckets_.Increment(-4, 2);
214+
std::make_unique<opentelemetry::sdk::metrics::AdaptingCircularBufferCounter>(10);
215+
histogram_point_data2.positive_buckets_->Increment(3, 1);
216+
histogram_point_data2.negative_buckets_->Increment(-2, 1);
217+
histogram_point_data2.negative_buckets_->Increment(-4, 2);
218218

219219
metric_sdk::ResourceMetrics data;
220220
auto resource = opentelemetry::sdk::resource::Resource::Create(

‎exporters/otlp/src/otlp_metric_utils.cc‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,11 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric(
199199
proto_histogram_point_data->set_time_unix_nano(ts);
200200
auto histogram_data = nostd::get<sdk::metrics::Base2ExponentialHistogramPointData>(
201201
point_data_with_attributes.point_data);
202+
if (histogram_data.positive_buckets_ == nullptr &&
203+
histogram_data.negative_buckets_ == nullptr)
204+
{
205+
continue;
206+
}
202207
// sum
203208
proto_histogram_point_data->set_sum(histogram_data.sum_);
204209
proto_histogram_point_data->set_count(histogram_data.count_);
@@ -208,27 +213,27 @@ void OtlpMetricUtils::ConvertExponentialHistogramMetric(
208213
proto_histogram_point_data->set_max(histogram_data.max_);
209214
}
210215
// negative buckets
211-
if (!histogram_data.negative_buckets_.Empty())
216+
if (!histogram_data.negative_buckets_->Empty())
212217
{
213218
auto negative_buckets = proto_histogram_point_data->mutable_negative();
214-
negative_buckets->set_offset(histogram_data.negative_buckets_.StartIndex());
219+
negative_buckets->set_offset(histogram_data.negative_buckets_->StartIndex());
215220

216-
for (auto index = histogram_data.negative_buckets_.StartIndex();
217-
index <= histogram_data.negative_buckets_.EndIndex(); ++index)
221+
for (auto index = histogram_data.negative_buckets_->StartIndex();
222+
index <= histogram_data.negative_buckets_->EndIndex(); ++index)
218223
{
219-
negative_buckets->add_bucket_counts(histogram_data.negative_buckets_.Get(index));
224+
negative_buckets->add_bucket_counts(histogram_data.negative_buckets_->Get(index));
220225
}
221226
}
222227
// positive buckets
223-
if (!histogram_data.positive_buckets_.Empty())
228+
if (!histogram_data.positive_buckets_->Empty())
224229
{
225230
auto positive_buckets = proto_histogram_point_data->mutable_positive();
226-
positive_buckets->set_offset(histogram_data.positive_buckets_.StartIndex());
231+
positive_buckets->set_offset(histogram_data.positive_buckets_->StartIndex());
227232

228-
for (auto index = histogram_data.positive_buckets_.StartIndex();
229-
index <= histogram_data.positive_buckets_.EndIndex(); ++index)
233+
for (auto index = histogram_data.positive_buckets_->StartIndex();
234+
index <= histogram_data.positive_buckets_->EndIndex(); ++index)
230235
{
231-
positive_buckets->add_bucket_counts(histogram_data.positive_buckets_.Get(index));
236+
positive_buckets->add_bucket_counts(histogram_data.positive_buckets_->Get(index));
232237
}
233238
}
234239
proto_histogram_point_data->set_scale(histogram_data.scale_);

‎sdk/include/opentelemetry/sdk/metrics/data/point_data.h‎

Lines changed: 67 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -69,33 +69,79 @@ class HistogramPointData
6969
class Base2ExponentialHistogramPointData
7070
{
7171
public:
72-
// Delete copy constructor and copy assignment operator
73-
Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &) = delete;
74-
Base2ExponentialHistogramPointData &operator=(const Base2ExponentialHistogramPointData &) =
75-
delete;
76-
77-
// Default move constructor and move assignment operator
7872
Base2ExponentialHistogramPointData(Base2ExponentialHistogramPointData &&) noexcept = default;
79-
Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept =
80-
default;
73+
Base2ExponentialHistogramPointData &operator=(Base2ExponentialHistogramPointData &&) noexcept = default;
74+
75+
Base2ExponentialHistogramPointData(const Base2ExponentialHistogramPointData &other)
76+
: sum_(other.sum_),
77+
min_(other.min_),
78+
max_(other.max_),
79+
zero_threshold_(other.zero_threshold_),
80+
count_(other.count_),
81+
zero_count_(other.zero_count_),
82+
max_buckets_(other.max_buckets_),
83+
scale_(other.scale_),
84+
record_min_max_(other.record_min_max_)
85+
{
86+
if (other.positive_buckets_)
87+
{
88+
positive_buckets_ = std::make_unique<AdaptingCircularBufferCounter>(*other.positive_buckets_);
89+
}
90+
if (other.negative_buckets_)
91+
{
92+
negative_buckets_ = std::make_unique<AdaptingCircularBufferCounter>(*other.negative_buckets_);
93+
}
94+
}
95+
96+
Base2ExponentialHistogramPointData &operator=(const Base2ExponentialHistogramPointData &other)
97+
{
98+
if (this != &other)
99+
{
100+
sum_ = other.sum_;
101+
min_ = other.min_;
102+
max_ = other.max_;
103+
zero_threshold_ = other.zero_threshold_;
104+
count_ = other.count_;
105+
zero_count_ = other.zero_count_;
106+
max_buckets_ = other.max_buckets_;
107+
scale_ = other.scale_;
108+
record_min_max_ = other.record_min_max_;
109+
110+
if (other.positive_buckets_)
111+
{
112+
positive_buckets_ = std::make_unique<AdaptingCircularBufferCounter>(*other.positive_buckets_);
113+
}
114+
else
115+
{
116+
positive_buckets_.reset();
117+
}
118+
119+
if (other.negative_buckets_)
120+
{
121+
negative_buckets_ = std::make_unique<AdaptingCircularBufferCounter>(*other.negative_buckets_);
122+
}
123+
else
124+
{
125+
negative_buckets_.reset();
126+
}
127+
}
128+
return *this;
129+
}
81130

82131
// Default constructor
83132
Base2ExponentialHistogramPointData() = default;
84133

85-
double sum_ = {};
86-
double min_ = {};
87-
double max_ = {};
134+
// Members
135+
double sum_ = {};
136+
double min_ = {};
137+
double max_ = {};
88138
double zero_threshold_ = {};
89-
uint64_t count_ = {};
90-
uint64_t zero_count_ = {};
91-
92-
std::unique_ptr<AdaptingCircularBufferCounter> positive_buckets_ =
93-
std::make_unique<AdaptingCircularBufferCounter>(0);
94-
std::unique_ptr<AdaptingCircularBufferCounter> negative_buckets_ =
95-
std::make_unique<AdaptingCircularBufferCounter>(0);
96-
97-
size_t max_buckets_ = {};
98-
int32_t scale_ = {};
139+
uint64_t count_ = {};
140+
uint64_t zero_count_ = {};
141+
std::unique_ptr<AdaptingCircularBufferCounter> positive_buckets_ = std::make_unique<AdaptingCircularBufferCounter>(0);
142+
std::unique_ptr<AdaptingCircularBufferCounter> negative_buckets_ = std::make_unique<AdaptingCircularBufferCounter>(0);
143+
size_t max_buckets_ = {};
144+
int32_t scale_ = {};
99145
bool record_min_max_ = true;
100146
};
101147

0 commit comments

Comments
 (0)