metrics: add per-version cumulative counters
This adds counters that accumulate some measurement across an OS version;
they are reset at version updates, but reported more frequently (for instance,
daily).
Such counts could be obtained by pseudonymous dremel queries, but this is
more convenient.
The code replaces the "tag" datum in the counters with two tags: a
"report tag" and a "reset tag". When the report
tag changes, the count is reported but not reset. When the reset tag
changes, the count is both reported and reset.
This also adds one usage of the new counter which tracks the total
number of kernel crashes since the most recent OS version update.
The state machine in counter.cc changes a bit because it's no longer
true that a counter is reset after reporting it. That logic is
still rather confusing, and could use a rewrite.
BUG=chromium:339588
TEST=ran on target under various situations
BRANCH=none
Change-Id: I5f83731e1a3d6e055b6d0f89111c9ffc60ccfcb9
Reviewed-on: https://chromium-review.googlesource.com/185081
Reviewed-by: Daniel Erat <derat@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Luigi Semenzato <semenzato@chromium.org>
diff --git a/metrics/counter_test.cc b/metrics/counter_test.cc
index c0761e2..90c3cd8 100644
--- a/metrics/counter_test.cc
+++ b/metrics/counter_test.cc
@@ -28,7 +28,8 @@
class RecordTest : public testing::Test {
protected:
virtual void SetUp() {
- EXPECT_EQ(0, record_.tag());
+ EXPECT_EQ(0, record_.report_tag());
+ EXPECT_EQ(0, record_.reset_tag());
EXPECT_EQ(0, record_.count());
}
@@ -62,11 +63,28 @@
base::DeleteFile(FilePath(kTestRecordFile), false);
}
- // Asserts that the record file contains the specified contents.
- testing::AssertionResult AssertRecord(const char* expr_tag,
+ testing::AssertionResult AssertRecord(const char* expr_reset_tag,
const char* expr_count,
- int32 expected_tag,
+ uint32 expected_reset_tag,
int32 expected_count) {
+ return AssertRecordFull(12345, expected_reset_tag, expected_count, false);
+ }
+
+ // Asserts that the record file contains the specified contents.
+ testing::AssertionResult AssertRecord3(const char* expr_report_tag,
+ const char* expr_reset_tag,
+ const char* expr_count,
+ uint32 expected_report_tag,
+ uint32 expected_reset_tag,
+ int32 expected_count) {
+ return AssertRecordFull(expected_report_tag, expected_reset_tag,
+ expected_count, true);
+ }
+
+ testing::AssertionResult AssertRecordFull(uint32 expected_report_tag,
+ uint32 expected_reset_tag,
+ int32 expected_count,
+ bool check_report_tag) {
int fd = HANDLE_EINTR(open(kTestRecordFile, O_RDONLY));
if (fd < 0) {
testing::Message msg;
@@ -84,10 +102,16 @@
return testing::AssertionFailure(msg);
}
- if (record.tag() != expected_tag || record.count() != expected_count) {
+ if ((check_report_tag && (record.report_tag() != expected_report_tag)) ||
+ record.reset_tag() != expected_reset_tag ||
+ record.count() != expected_count) {
testing::Message msg;
- msg << "actual record (" << record.tag() << ", " << record.count()
- << ") expected (" << expected_tag << ", " << expected_count << ")";
+ msg << "actual record (" << record.report_tag() << ", "
+ << record.reset_tag() << ", " << record.count()
+ << ") expected (" << expected_report_tag << ", "
+ << expected_reset_tag << ", " << expected_count << ")";
+ if (!check_report_tag)
+ msg << "\n(ignore differences in the first field)";
HANDLE_EINTR(close(fd));
return testing::AssertionFailure(msg);
}
@@ -99,7 +123,7 @@
// Returns true if the persistent record file does not exist or is
// empty, false otherwise.
bool AssertNoOrEmptyRecordFile() {
- FilePath record_file(counter_.filename_);
+ base::FilePath record_file(counter_.filename_);
int64 record_file_size;
return !base::PathExists(record_file) ||
(base::GetFileSize(record_file, &record_file_size) &&
@@ -108,18 +132,18 @@
// Adds a reporter call expectation that the specified tag/count
// callback will be generated.
- void ExpectReporterCall(int32 tag, int32 count) {
- EXPECT_CALL(reporter_, Call(_, tag, count))
+ void ExpectReporterCall(int32 count) {
+ EXPECT_CALL(reporter_, Call(_, count))
.Times(1)
.RetiresOnSaturation();
}
// The reporter callback forwards the call to the reporter mock so
// that we can set call expectations.
- static void Reporter(void* handle, int32 tag, int32 count) {
+ static void Reporter(void* handle, int32 count) {
TaggedCounterTest* test = static_cast<TaggedCounterTest*>(handle);
ASSERT_FALSE(NULL == test);
- test->reporter_.Call(handle, tag, count);
+ test->reporter_.Call(handle, count);
}
// Collects log messages in the |log_| member string so that they
@@ -148,8 +172,7 @@
std::string log_;
// Reporter mock to set callback expectations on.
- StrictMock<MockFunction<void(void* handle,
- int32 tag, int32 count)> > reporter_;
+ StrictMock<MockFunction<void(void* handle, int32 count)> > reporter_;
// Pointer to the current test fixture.
static TaggedCounterTest* test_;
@@ -159,12 +182,14 @@
TaggedCounterTest* TaggedCounterTest::test_ = NULL;
TEST_F(RecordTest, Init) {
- record_.Init(/* tag */ 5, /* count */ -1);
- EXPECT_EQ(5, record_.tag());
+ record_.Init(/* report_tag */ 8, /* reset_tag */ 5, /* count */ -1);
+ EXPECT_EQ(8, record_.report_tag());
+ EXPECT_EQ(5, record_.reset_tag());
EXPECT_EQ(0, record_.count());
- record_.Init(/* tag */ -2, /* count */ 10);
- EXPECT_EQ(-2, record_.tag());
+ record_.Init(/* report_tag */ -8, /* reset_tag */ -2, /* count */ 10);
+ EXPECT_EQ(-8, record_.report_tag());
+ EXPECT_EQ(-2, record_.reset_tag());
EXPECT_EQ(10, record_.count());
}
@@ -193,7 +218,7 @@
// created.
counter_.Init(kDoesNotExistFile,
/* reporter */ NULL, /* reporter_handle */ NULL);
- counter_.Update(/* tag */ 10, /* count */ 20);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 10, /* count */ 20);
EXPECT_TRUE(LogContains("Unable to open the persistent counter file: "
"No such file or directory"));
EXPECT_EQ(TaggedCounter::kRecordInvalid, counter_.record_state_);
@@ -204,62 +229,72 @@
counter_.Flush();
EXPECT_EQ(TaggedCounter::kRecordNull, counter_.record_state_);
- counter_.Update(/* tag */ 40, /* count */ 60);
- ExpectReporterCall(/* tag */ 40, /* count */ 60);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 40, /* count */ 60);
+ ExpectReporterCall(/* count */ 60);
counter_.Flush();
EXPECT_TRUE(AssertNoOrEmptyRecordFile());
EXPECT_EQ(TaggedCounter::kRecordNull, counter_.record_state_);
- counter_.Update(/* tag */ 41, /* count */ 70);
- counter_.record_.Init(/* tag */ 0, /* count */ 0);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 41, /* count */ 70);
+ counter_.record_.Init(/* report_tag */ 0, /* reset_tag */ 0, /* count */ 0);
counter_.record_state_ = TaggedCounter::kRecordInvalid;
- ExpectReporterCall(/* tag */ 41, /* count */ 70);
+ ExpectReporterCall(/* count */ 70);
counter_.Flush();
EXPECT_TRUE(AssertNoOrEmptyRecordFile());
EXPECT_EQ(TaggedCounter::kRecordNull, counter_.record_state_);
}
TEST_F(TaggedCounterTest, InitFromFile) {
- counter_.Update(/* tag */ 30, /* count */ 50);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 30, /* count */ 50);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 30, /* seconds */ 50);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
counter_.Init(kTestRecordFile, &Reporter, this);
- counter_.Update(/* tag */ 30, /* count */ 40);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 30, /* count */ 40);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 30, /* seconds */ 90);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
counter_.Init(kTestRecordFile, &Reporter, this);
- ExpectReporterCall(/* tag */ 30, /* count */ 90);
- counter_.Update(/* tag */ 31, /* count */ 60);
+ ExpectReporterCall(/* count */ 90);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 31, /* count */ 60);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 31, /* seconds */ 60);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
- ExpectReporterCall(/* tag */ 31, /* count */ 60);
+ ExpectReporterCall(/* count */ 60);
counter_.Init(kTestRecordFile, &Reporter, this);
- counter_.Update(/* tag */ 32, /* count */ 0);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 32, /* count */ 0);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 32, /* seconds */ 0);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
}
TEST_F(TaggedCounterTest, Update) {
- counter_.Update(/* tag */ 20, /* count */ 30);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 20, /* count */ 30);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 20, /* seconds */ 30);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
- counter_.Update(/* tag */ 20, /* count */ 40);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 20, /* count */ 40);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 20, /* seconds */ 70);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
- ExpectReporterCall(/* tag */ 20, /* count */ 70);
- counter_.Update(/* tag */ 21, /* count */ 15);
+ ExpectReporterCall(/* count */ 70);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 21, /* count */ 15);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 21, /* seconds */ 15);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
- ExpectReporterCall(/* tag */ 21, /* count */ 15);
- counter_.Update(/* tag */ 22, /* count */ 0);
+ ExpectReporterCall(/* count */ 15);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 22, /* count */ 0);
EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 22, /* seconds */ 0);
EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
+
+ ExpectReporterCall(/* count */ 33);
+ counter_.Update(/* report_tag */ 0, /* reset_tag */ 22, /* count */ 33);
+ EXPECT_PRED_FORMAT2(AssertRecord, /* day */ 22, /* seconds */ 33);
+ EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
+ // Check that changing the report tag does not reset the counter.
+ counter_.Update(/* report_tag */ 1, /* reset_tag */ 22, /* count */ 0);
+ EXPECT_PRED_FORMAT3(AssertRecord3, /* version */ 1,
+ /* day */ 22, /* seconds */ 33);
+ EXPECT_EQ(TaggedCounter::kRecordValid, counter_.record_state_);
}
static const char kTestFilename[] = "test_filename";
@@ -311,10 +346,10 @@
TEST_F(TaggedCounterReporterTest, Update) {
DoInit();
- EXPECT_CALL(*tagged_counter_, Update(1, 2))
+ EXPECT_CALL(*tagged_counter_, Update(1, 0, 2))
.Times(1)
.RetiresOnSaturation();
- reporter_.Update(1, 2);
+ reporter_.Update(1, 0, 2);
}
TEST_F(TaggedCounterReporterTest, Flush) {
@@ -334,7 +369,7 @@
kHistogramBuckets))
.Times(1)
.RetiresOnSaturation();
- reporter_.Report(&reporter_, 127, 301);
+ reporter_.Report(&reporter_, 301);
}
class FrequencyCounterTest : public testing::Test {
@@ -385,12 +420,39 @@
TEST_F(FrequencyCounterTest, UpdateInternal) {
CheckInit(kSecondsPerWeek);
- EXPECT_CALL(*tagged_counter_, Update(150, 2))
+ EXPECT_CALL(*tagged_counter_, Update(0, 150, 2))
.Times(1)
.RetiresOnSaturation();
frequency_counter_.UpdateInternal(2, kSecondsPerWeek * 150);
}
+class VersionCounterTest : public testing::Test {
+ protected:
+ virtual void SetUp() {
+ tagged_counter_ = NULL;
+ }
+ void Init();
+
+ VersionCounter version_counter_;
+ StrictMock<TaggedCounterMock>* tagged_counter_;
+
+ TaggedCounter::Reporter reporter_;
+};
+
+void VersionCounterTest::Init() {
+ tagged_counter_ = new StrictMock<TaggedCounterMock>;
+ version_counter_.Init(tagged_counter_, 1);
+ EXPECT_EQ(tagged_counter_, version_counter_.tagged_counter_.get());
+}
+
+TEST_F(VersionCounterTest, UpdateInternal) {
+ Init();
+ EXPECT_CALL(*tagged_counter_, Update(0, 150, 2))
+ .Times(1)
+ .RetiresOnSaturation();
+ version_counter_.UpdateInternal(2, 0, 150);
+}
+
} // namespace chromeos_metrics
int main(int argc, char** argv) {