Skip to content
This repository was archived by the owner on Jul 31, 2023. It is now read-only.

Commit f718f3d

Browse files
g-easyg-easy
authored
When getting ViewData, set the end_time to the current time. (#395)
This is to make sure exporters will export what the value is currently, not what it was the last time a view was touched. Move end_time from ViewDataImpl to ViewData. No API change.
1 parent 64567d5 commit f718f3d

5 files changed

Lines changed: 15 additions & 33 deletions

File tree

‎opencensus/stats/internal/view_data.cc‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include "absl/base/macros.h"
2020
#include "absl/memory/memory.h"
21+
#include "absl/time/clock.h"
2122
#include "opencensus/stats/internal/view_data_impl.h"
2223

2324
namespace opencensus {
@@ -80,13 +81,14 @@ const ViewData::DataMap<Distribution>& ViewData::distribution_data() const {
8081
}
8182

8283
absl::Time ViewData::start_time() const { return impl_->start_time(); }
83-
absl::Time ViewData::end_time() const { return impl_->end_time(); }
84+
absl::Time ViewData::end_time() const { return end_time_; }
8485

8586
ViewData::ViewData(const ViewData& other)
86-
: impl_(absl::make_unique<ViewDataImpl>(*other.impl_)) {}
87+
: impl_(absl::make_unique<ViewDataImpl>(*other.impl_)),
88+
end_time_(absl::Now()) {}
8789

8890
ViewData::ViewData(std::unique_ptr<ViewDataImpl> data)
89-
: impl_(std::move(data)) {
91+
: impl_(std::move(data)), end_time_(absl::Now()) {
9092
ABSL_ASSERT(impl_->type() != ViewDataImpl::Type::kStatsObject);
9193
}
9294

‎opencensus/stats/internal/view_data_impl.cc‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ ViewDataImpl::ViewDataImpl(const ViewDataImpl& other, absl::Time now)
9292
? Type::kDistribution
9393
: Type::kDouble),
9494
start_time_(std::max(other.start_time(),
95-
now - other.aggregation_window().duration())),
96-
end_time_(now) {
95+
now - other.aggregation_window().duration())) {
9796
ABSL_ASSERT(aggregation_window_.type() == AggregationWindow::Type::kInterval);
9897
switch (aggregation_.type()) {
9998
case Aggregation::Type::kSum:
@@ -157,8 +156,7 @@ ViewDataImpl::ViewDataImpl(const ViewDataImpl& other)
157156
: aggregation_(other.aggregation_),
158157
aggregation_window_(other.aggregation_window_),
159158
type_(other.type()),
160-
start_time_(other.start_time_),
161-
end_time_(other.end_time_) {
159+
start_time_(other.start_time_) {
162160
switch (type_) {
163161
case Type::kDouble: {
164162
new (&double_data_) DataMap<double>(other.double_data_);
@@ -184,7 +182,6 @@ ViewDataImpl::ViewDataImpl(const ViewDataImpl& other)
184182

185183
void ViewDataImpl::Merge(const std::vector<std::string>& tag_values,
186184
const MeasureData& data, absl::Time now) {
187-
end_time_ = std::max(end_time_, now);
188185
switch (type_) {
189186
case Type::kDouble: {
190187
if (aggregation_.type() == Aggregation::Type::kSum) {
@@ -259,8 +256,7 @@ ViewDataImpl::ViewDataImpl(ViewDataImpl* source, absl::Time now)
259256
: aggregation_(source->aggregation_),
260257
aggregation_window_(source->aggregation_window_),
261258
type_(source->type_),
262-
start_time_(source->start_time_),
263-
end_time_(now) {
259+
start_time_(source->start_time_) {
264260
switch (type_) {
265261
case Type::kDouble: {
266262
new (&double_data_) DataMap<double>();
@@ -285,7 +281,6 @@ ViewDataImpl::ViewDataImpl(ViewDataImpl* source, absl::Time now)
285281
}
286282
}
287283
source->start_time_ = now;
288-
source->end_time_ = now;
289284
}
290285

291286
} // namespace stats

‎opencensus/stats/internal/view_data_impl.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ class ViewDataImpl {
104104
}
105105

106106
absl::Time start_time() const { return start_time_; }
107-
absl::Time end_time() const { return end_time_; }
108107

109108
// Merges bulk data for the given tag values at 'now'. tag_values must be
110109
// ordered according to the order of keys in the ViewDescriptor.
@@ -131,7 +130,6 @@ class ViewDataImpl {
131130
DataMap<IntervalStatsObject> interval_data_;
132131
};
133132
absl::Time start_time_;
134-
absl::Time end_time_;
135133
};
136134

137135
} // namespace stats

‎opencensus/stats/internal/view_data_impl_test.cc‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ TEST(ViewDataImplTest, Sum) {
5555
EXPECT_EQ(Aggregation::Sum(), data.aggregation());
5656
EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
5757
EXPECT_EQ(start_time, data.start_time());
58-
EXPECT_EQ(end_time, data.end_time());
5958
EXPECT_THAT(data.double_data(),
6059
::testing::UnorderedElementsAre(::testing::Pair(tags1, 3),
6160
::testing::Pair(tags2, 5)));
@@ -77,7 +76,6 @@ TEST(ViewDataImplTest, Count) {
7776
EXPECT_EQ(Aggregation::Count(), data.aggregation());
7877
EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
7978
EXPECT_EQ(start_time, data.start_time());
80-
EXPECT_EQ(end_time, data.end_time());
8179
EXPECT_THAT(data.int_data(),
8280
::testing::UnorderedElementsAre(::testing::Pair(tags1, 2),
8381
::testing::Pair(tags2, 1)));
@@ -100,7 +98,6 @@ TEST(ViewDataImplTest, Distribution) {
10098
EXPECT_EQ(Aggregation::Distribution(buckets), data.aggregation());
10199
EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
102100
EXPECT_EQ(start_time, data.start_time());
103-
EXPECT_EQ(end_time, data.end_time());
104101
EXPECT_EQ(data.distribution_data().size(), 2);
105102
EXPECT_THAT(data.distribution_data().find(tags1)->second.bucket_counts(),
106103
::testing::ElementsAre(2, 0));
@@ -127,7 +124,6 @@ TEST(ViewDataImplTest, LastValueDouble) {
127124
EXPECT_EQ(Aggregation::LastValue(), data.aggregation());
128125
EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
129126
EXPECT_EQ(start_time, data.start_time());
130-
EXPECT_EQ(end_time, data.end_time());
131127
EXPECT_THAT(data.double_data(),
132128
::testing::UnorderedElementsAre(::testing::Pair(tags1, 5.0),
133129
::testing::Pair(tags2, 15.0)));
@@ -152,7 +148,6 @@ TEST(ViewDataImplTest, LastValueInt64) {
152148
EXPECT_EQ(Aggregation::LastValue(), data.aggregation());
153149
EXPECT_EQ(AggregationWindow::Cumulative(), data.aggregation_window());
154150
EXPECT_EQ(start_time, data.start_time());
155-
EXPECT_EQ(end_time, data.end_time());
156151
EXPECT_THAT(data.int_data(),
157152
::testing::UnorderedElementsAre(::testing::Pair(tags1, 5),
158153
::testing::Pair(tags2, 15)));
@@ -179,15 +174,12 @@ TEST(ViewDataImplTest, StatsObjectToCount) {
179174
EXPECT_EQ(AggregationWindow::Interval(interval),
180175
export_data1.aggregation_window());
181176
EXPECT_EQ(start_time, export_data1.start_time());
182-
EXPECT_EQ(time, export_data1.end_time());
183177
EXPECT_THAT(export_data1.double_data(),
184178
::testing::UnorderedElementsAre(::testing::Pair(tags1, 3),
185179
::testing::Pair(tags2, 1)));
186180

187-
time += interval;
188-
const ViewDataImpl export_data2(data, time);
189-
EXPECT_EQ(time - interval, export_data2.start_time());
190-
EXPECT_EQ(time, export_data2.end_time());
181+
const ViewDataImpl export_data2(data, time + interval);
182+
EXPECT_EQ(time, export_data2.start_time());
191183
EXPECT_THAT(export_data2.double_data(),
192184
::testing::UnorderedElementsAre(::testing::Pair(tags1, 1),
193185
::testing::Pair(tags2, 0)));
@@ -214,15 +206,12 @@ TEST(ViewDataImplTest, StatsObjectToSum) {
214206
EXPECT_EQ(AggregationWindow::Interval(interval),
215207
export_data1.aggregation_window());
216208
EXPECT_EQ(start_time, export_data1.start_time());
217-
EXPECT_EQ(time, export_data1.end_time());
218209
EXPECT_THAT(export_data1.double_data(),
219210
::testing::UnorderedElementsAre(::testing::Pair(tags1, 6),
220211
::testing::Pair(tags2, 2)));
221212

222-
time += interval;
223-
const ViewDataImpl export_data2(data, time);
224-
EXPECT_EQ(time - interval, export_data2.start_time());
225-
EXPECT_EQ(time, export_data2.end_time());
213+
const ViewDataImpl export_data2(data, time + interval);
214+
EXPECT_EQ(time, export_data2.start_time());
226215
EXPECT_THAT(export_data2.double_data(),
227216
::testing::UnorderedElementsAre(::testing::Pair(tags1, 2),
228217
::testing::Pair(tags2, 0)));
@@ -251,7 +240,6 @@ TEST(ViewDataImplTest, StatsObjectToDistribution) {
251240
EXPECT_EQ(AggregationWindow::Interval(interval),
252241
export_data1.aggregation_window());
253242
EXPECT_EQ(start_time, export_data1.start_time());
254-
EXPECT_EQ(time, export_data1.end_time());
255243
EXPECT_EQ(2, export_data1.distribution_data().size());
256244
const Distribution& distribution_1_1 =
257245
export_data1.distribution_data().find(tags1)->second;
@@ -270,10 +258,8 @@ TEST(ViewDataImplTest, StatsObjectToDistribution) {
270258
EXPECT_EQ(0, distribution_2_1.max());
271259
EXPECT_THAT(distribution_2_1.bucket_counts(), ::testing::ElementsAre(1, 0));
272260

273-
time += interval;
274-
const ViewDataImpl export_data2(data, time);
275-
EXPECT_EQ(time - interval, export_data2.start_time());
276-
EXPECT_EQ(time, export_data2.end_time());
261+
const ViewDataImpl export_data2(data, time + interval);
262+
EXPECT_EQ(time, export_data2.start_time());
277263
EXPECT_EQ(2, export_data2.distribution_data().size());
278264
const Distribution& distribution_1_2 =
279265
export_data2.distribution_data().find(tags1)->second;

‎opencensus/stats/view_data.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ class ViewData {
8080
explicit ViewData(std::unique_ptr<ViewDataImpl> data);
8181

8282
const std::unique_ptr<ViewDataImpl> impl_;
83+
const absl::Time end_time_;
8384
};
8485

8586
} // namespace stats

0 commit comments

Comments
 (0)