commit 328e19d119f3221e3c3706c7785853cc5e8339af
author Darin Petkov <petkov@chromium.org> Tue Dec 04 15:54:07 2012 +0100
committer Gerrit <chrome-bot@google.com> Wed Dec 05 05:12:32 2012 -0800
tree f38f2dcd098cfbb441d9ba43ae49ffca2f0cd155
parent 385b9bccbcdb5395e1fc8903a0f14cb6016b01c3

shill: Throttle diagnostics log stashing (and uploading).

BUG=chromium-os:36775
TEST=unit tests

Change-Id: Ibb050cf7422a246dd903bfb39cb321105eaa02a1
Reviewed-on: https://gerrit.chromium.org/gerrit/39187
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Ready: Darin Petkov <petkov@chromium.org>

diagnostics_reporter.cc

diff --git a/diagnostics_reporter.cc b/diagnostics_reporter.cc
index e1bb730..8f730be 100644
--- a/diagnostics_reporter.cc
+++ b/diagnostics_reporter.cc
@@ -5,6 +5,7 @@
 #include "shill/diagnostics_reporter.h"
 
 #include "shill/glib.h"
+#include "shill/shill_time.h"
 
 namespace shill {
 
@@ -15,7 +16,13 @@
 
 }  // namespace
 
-DiagnosticsReporter::DiagnosticsReporter() : glib_(NULL) {}
+// static
+const int DiagnosticsReporter::kLogStashThrottleSeconds = 30 * 60;
+
+DiagnosticsReporter::DiagnosticsReporter()
+    : glib_(NULL),
+      time_(Time::GetInstance()),
+      last_log_stash_(0) {}
 
 DiagnosticsReporter::~DiagnosticsReporter() {}
 
@@ -56,7 +63,15 @@
 void DiagnosticsReporter::OnConnectivityEvent() {
   LOG(INFO) << "Diagnostics event triggered.";
 
-  // TODO(petkov): Throttle log stashing (crosbug.com/36775).
+  struct timeval now = (const struct timeval){ 0 };
+  time_->GetTimeMonotonic(&now);
+  if (last_log_stash_ + kLogStashThrottleSeconds >
+      static_cast<uint64>(now.tv_sec)) {
+    LOG(INFO) << "Diagnostics throttled.";
+    return;
+  }
+
+  last_log_stash_ = now.tv_sec;
 
   // TODO(petkov): Stash away logs for potential inclusion in feedback
   // (crosbug.com/36923).

diagnostics_reporter.h

diff --git a/diagnostics_reporter.h b/diagnostics_reporter.h
index 22a9beb..cb0b1bc 100644
--- a/diagnostics_reporter.h
+++ b/diagnostics_reporter.h
@@ -10,6 +10,7 @@
 namespace shill {
 
 class GLib;
+class Time;
 
 class DiagnosticsReporter {
  public:
@@ -36,7 +37,11 @@
   friend struct base::DefaultLazyInstanceTraits<DiagnosticsReporter>;
   friend class DiagnosticsReporterTest;
 
+  static const int kLogStashThrottleSeconds;
+
   GLib *glib_;
+  Time *time_;
+  uint64 last_log_stash_;  // Monotonic time seconds.
 
   DISALLOW_COPY_AND_ASSIGN(DiagnosticsReporter);
 };

diagnostics_reporter_unittest.cc

diff --git a/diagnostics_reporter_unittest.cc b/diagnostics_reporter_unittest.cc
index e0c82e3..3332e51 100644
--- a/diagnostics_reporter_unittest.cc
+++ b/diagnostics_reporter_unittest.cc
@@ -8,24 +8,14 @@
 #include <gtest/gtest.h>
 
 #include "shill/mock_glib.h"
+#include "shill/mock_time.h"
 
 using testing::_;
 using testing::Return;
+using testing::SetArgumentPointee;
 
 namespace shill {
 
-class DiagnosticsReporterTest : public testing::Test {
- public:
-  DiagnosticsReporterTest() {}
-
- protected:
-  bool IsReportingEnabled() {
-    return DiagnosticsReporter::GetInstance()->IsReportingEnabled();
-  }
-
-  MockGLib glib_;
-};
-
 namespace {
 
 class ReporterUnderTest : public DiagnosticsReporter {
@@ -42,24 +32,71 @@
 
 }  // namespace
 
+class DiagnosticsReporterTest : public testing::Test {
+ public:
+  DiagnosticsReporterTest() {
+    reporter_.time_ = &time_;
+    reporter_.Init(&glib_);
+  }
+
+ protected:
+  bool IsReportingEnabled() {
+    return DiagnosticsReporter::GetInstance()->IsReportingEnabled();
+  }
+
+  void SetLastLogStash(uint64 time) { reporter_.last_log_stash_ = time; }
+  uint64 GetLastLogStash() { return reporter_.last_log_stash_; }
+  uint64 GetLogStashThrottleSeconds() {
+    return DiagnosticsReporter::kLogStashThrottleSeconds;
+  }
+
+  MockGLib glib_;
+  MockTime time_;
+  ReporterUnderTest reporter_;
+};
+
 TEST_F(DiagnosticsReporterTest, ReportEnabled) {
-  ReporterUnderTest reporter;
-  EXPECT_CALL(reporter, IsReportingEnabled()).WillOnce(Return(true));
+  EXPECT_CALL(reporter_, IsReportingEnabled()).WillOnce(Return(true));
   EXPECT_CALL(glib_, SpawnSync(_, _, _, _, _, _, _, _, _, _)).Times(1);
-  reporter.Init(&glib_);
-  reporter.Report();
+  reporter_.Report();
 }
 
 TEST_F(DiagnosticsReporterTest, ReportDisabled) {
-  ReporterUnderTest reporter;
-  EXPECT_CALL(reporter, IsReportingEnabled()).WillOnce(Return(false));
+  EXPECT_CALL(reporter_, IsReportingEnabled()).WillOnce(Return(false));
   EXPECT_CALL(glib_, SpawnSync(_, _, _, _, _, _, _, _, _, _)).Times(0);
-  reporter.Init(&glib_);
-  reporter.Report();
+  reporter_.Report();
 }
 
 TEST_F(DiagnosticsReporterTest, IsReportingEnabled) {
   EXPECT_FALSE(IsReportingEnabled());
 }
 
+TEST_F(DiagnosticsReporterTest, OnConnectivityEventThrottle) {
+  const uint64 kLastStash = 50;
+  const uint64 kNow = kLastStash + GetLogStashThrottleSeconds() - 1;
+  SetLastLogStash(kLastStash);
+  const struct timeval now = {
+    .tv_sec = static_cast<long int>(kNow),
+    .tv_usec = 0
+  };
+  EXPECT_CALL(time_, GetTimeMonotonic(_))
+      .WillOnce(DoAll(SetArgumentPointee<0>(now), Return(0)));
+  reporter_.OnConnectivityEvent();
+  EXPECT_EQ(kLastStash, GetLastLogStash());
+}
+
+TEST_F(DiagnosticsReporterTest, OnConnectivityEvent) {
+  const uint64 kLastStash = 50;
+  const uint64 kNow = kLastStash + GetLogStashThrottleSeconds() + 1;
+  SetLastLogStash(kLastStash);
+  const struct timeval now = {
+    .tv_sec = static_cast<long int>(kNow),
+    .tv_usec = 0
+  };
+  EXPECT_CALL(time_, GetTimeMonotonic(_))
+      .WillOnce(DoAll(SetArgumentPointee<0>(now), Return(0)));
+  reporter_.OnConnectivityEvent();
+  EXPECT_EQ(kNow, GetLastLogStash());
+}
+
 }  // namespace shill